Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fxied declaration emit for JS files when module.exports is assigned a non-alias value and when it has extra type members #56243

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8732,7 +8732,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
});
let addingDeclare = !bundled;
const exportEquals = symbolTable.get(InternalSymbolName.ExportEquals);
if (exportEquals && symbolTable.size > 1 && exportEquals.flags & SymbolFlags.Alias) {
if (exportEquals && symbolTable.size > 1 && exportEquals.flags & (SymbolFlags.Alias | SymbolFlags.Module)) {
symbolTable = createSymbolTable();
// Remove extraneous elements from root symbol table (they'll be mixed back in when the target of the `export=` is looked up)
symbolTable.set(InternalSymbolName.ExportEquals, exportEquals);
Expand Down Expand Up @@ -9265,8 +9265,18 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function getNamespaceMembersForSerialization(symbol: Symbol) {
const exports = getExportsOfSymbol(symbol);
return !exports ? [] : filter(arrayFrom(exports.values()), m => isNamespaceMember(m) && isIdentifierText(m.escapedName as string, ScriptTarget.ESNext));
let exports = arrayFrom(getExportsOfSymbol(symbol).values());
const merged = getMergedSymbol(symbol);
if (merged !== symbol) {
const membersSet = new Set(exports);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could potentially have the same symbol in both exports and the merged exports through some complicated aliasing. In such a case, it'd behoove us to deduplicate them. Maybe we could just add 'em both to a Set and then arrayifying it? The deduplicate array helper is... icky - definitely not for things as potentially large as a symbol export table.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i pushed out the requested change

for (const exported of getExportsOfSymbol(merged).values()) {
if (!(getSymbolFlags(resolveSymbol(exported)) & SymbolFlags.Value)) {
membersSet.add(exported);
}
}
exports = arrayFrom(membersSet);
}
return filter(exports, m => isNamespaceMember(m) && isIdentifierText(m.escapedName as string, ScriptTarget.ESNext));
}

function isTypeOnlyNamespace(symbol: Symbol) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [tests/cases/compiler/jsDeclarationEmitExportAssignedFunctionWithExtraTypedefsMembers.ts] ////

//// [index.js]
/**
* @typedef Options
* @property {string} opt
*/

/**
* @param {Options} options
*/
module.exports = function loader(options) {}


//// [index.js]
"use strict";
/**
* @typedef Options
* @property {string} opt
*/
/**
* @param {Options} options
*/
module.exports = function loader(options) { };


//// [index.d.ts]
declare namespace _exports {
export { Options };
}
declare function _exports(options: Options): void;
export = _exports;
type Options = {
opt: string;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//// [tests/cases/compiler/jsDeclarationEmitExportAssignedFunctionWithExtraTypedefsMembers.ts] ////

=== index.js ===
/**
* @typedef Options
* @property {string} opt
*/

/**
* @param {Options} options
*/
module.exports = function loader(options) {}
>module.exports : Symbol(module.exports, Decl(index.js, 0, 0))
>module : Symbol(export=, Decl(index.js, 0, 0))
>exports : Symbol(export=, Decl(index.js, 0, 0))
>loader : Symbol(loader, Decl(index.js, 8, 16))
>options : Symbol(options, Decl(index.js, 8, 33))

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//// [tests/cases/compiler/jsDeclarationEmitExportAssignedFunctionWithExtraTypedefsMembers.ts] ////

=== index.js ===
/**
* @typedef Options
* @property {string} opt
*/

/**
* @param {Options} options
*/
module.exports = function loader(options) {}
>module.exports = function loader(options) {} : (options: Options) => void
>module.exports : (options: Options) => void
>module : { exports: (options: Options) => void; }
>exports : (options: Options) => void
>function loader(options) {} : (options: Options) => void
>loader : (options: Options) => void
>options : Options

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @strict: true
// @checkJs: true
// @declaration: true
// @outDir: out

// @filename: index.js

/**
* @typedef Options
* @property {string} opt
*/

/**
* @param {Options} options
*/
module.exports = function loader(options) {}
Loading