Skip to content

Don't emit redundant and invalid extra renamed keyword exports in JS files #62104

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
20 changes: 1 addition & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9552,11 +9552,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
symbol.flags & SymbolFlags.ExportDoesNotSupportDefaultModifier
|| (symbol.flags & SymbolFlags.Function && length(getPropertiesOfType(getTypeOfSymbol(symbol))))
) && !(symbol.flags & SymbolFlags.Alias); // An alias symbol should preclude needing to make an alias ourselves
let needsExportDeclaration = !needsPostExportDefault && !isPrivate && isStringANonContextualKeyword(symbolName) && !isDefault;
// `serializeVariableOrProperty` will handle adding the export declaration if it is run (since `getInternalSymbolName` will create the name mapping), so we need to ensuer we unset `needsExportDeclaration` if it is
if (needsPostExportDefault || needsExportDeclaration) {
isPrivate = true;
}
isPrivate ||= needsPostExportDefault;
const modifierFlags = (!isPrivate ? ModifierFlags.Export : 0) | (isDefault && !needsPostExportDefault ? ModifierFlags.Default : 0);
const isConstMergedWithNS = symbol.flags & SymbolFlags.Module &&
symbol.flags & (SymbolFlags.BlockScopedVariable | SymbolFlags.FunctionScopedVariable | SymbolFlags.Property) &&
Expand All @@ -9581,7 +9577,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (propertyAsAlias) {
const createdExport = serializeMaybeAliasAssignment(symbol);
if (createdExport) {
needsExportDeclaration = false;
needsPostExportDefault = false;
}
}
Expand Down Expand Up @@ -9679,7 +9674,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
),
ModifierFlags.None,
);
needsExportDeclaration = false;
needsPostExportDefault = false;
}
}
Expand Down Expand Up @@ -9737,18 +9731,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
context.approximateLength += 16 + internalSymbolName.length; // `export default internalName;`
addResult(factory.createExportAssignment(/*modifiers*/ undefined, /*isExportEquals*/ false, factory.createIdentifier(internalSymbolName)), ModifierFlags.None);
}
else if (needsExportDeclaration) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was originally added by @weswigham in #38982

I tried to think of a potential missing test case, but I couldn't find one. Perhaps this just became handled by other code paths since this was introduced

const internalSymbolName = getInternalSymbolName(symbol, symbolName);
context.approximateLength += 22 + symbolName.length + internalSymbolName.length; // `export { internalName as symbolName };`
addResult(
factory.createExportDeclaration(
/*modifiers*/ undefined,
/*isTypeOnly*/ false,
factory.createNamedExports([factory.createExportSpecifier(/*isTypeOnly*/ false, internalSymbolName, symbolName)]),
),
ModifierFlags.None,
);
}
}

function includePrivateSymbol(symbol: Symbol) {
Expand Down
26 changes: 26 additions & 0 deletions tests/baselines/reference/jsDeclarationsKeywordExport1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsKeywordExport1.ts] ////

//// [source.js]
// https://github.com/microsoft/TypeScript/issues/62081

const _null = {}
const $void = {}

export { _null as null, $void as void }


//// [source.js]
"use strict";
// https://github.com/microsoft/TypeScript/issues/62081
Object.defineProperty(exports, "__esModule", { value: true });
exports.void = exports.null = void 0;
var _null = {};
exports.null = _null;
var $void = {};
exports.void = $void;


//// [source.d.ts]
declare const _null: {};
declare const $void: {};
export { _null as null, $void as void };
17 changes: 17 additions & 0 deletions tests/baselines/reference/jsDeclarationsKeywordExport1.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsKeywordExport1.ts] ////

=== source.js ===
// https://github.com/microsoft/TypeScript/issues/62081

const _null = {}
>_null : Symbol(_null, Decl(source.js, 2, 5))

const $void = {}
>$void : Symbol($void, Decl(source.js, 3, 5))

export { _null as null, $void as void }
>_null : Symbol(_null, Decl(source.js, 2, 5))
>null : Symbol(null, Decl(source.js, 5, 8))
>$void : Symbol($void, Decl(source.js, 3, 5))
>void : Symbol(void, Decl(source.js, 5, 23))

27 changes: 27 additions & 0 deletions tests/baselines/reference/jsDeclarationsKeywordExport1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//// [tests/cases/conformance/jsdoc/declarations/jsDeclarationsKeywordExport1.ts] ////

=== source.js ===
// https://github.com/microsoft/TypeScript/issues/62081

const _null = {}
>_null : {}
> : ^^
>{} : {}
> : ^^

const $void = {}
>$void : {}
> : ^^
>{} : {}
> : ^^

export { _null as null, $void as void }
>_null : {}
> : ^^
>null : {}
> : ^^
>$void : {}
> : ^^
>void : {}
> : ^^

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @allowJs: true
// @checkJs: true
// @target: es5
// @outDir: ./out
// @declaration: true
// @filename: source.js

// https://github.com/microsoft/TypeScript/issues/62081

const _null = {}
const $void = {}

export { _null as null, $void as void }
Loading