Exports aliases for CJS exports.Foo = ... assignments when appropriate#3431
Exports aliases for CJS exports.Foo = ... assignments when appropriate#3431ahejlsberg merged 4 commits intomainfrom
exports.Foo = ... assignments when appropriate#3431Conversation
There was a problem hiding this comment.
Pull request overview
Adds CommonJS-export alias support so exports.Foo = <aliasable expression> can preserve type/namespace meanings (similar to export { C as Foo }), improving typechecking and declaration emit for many JS/CJS patterns in the TypeScript-Go compiler/LS.
Changes:
- Emit
export { source as name }-style declarations for eligible CommonJS export property assignments in the declaration transformer. - Treat
exports.<prop> = <alias>assignments as CommonJS module exports for alias marking/visibility and alias-declaration detection. - Update a large set of submodule reference baselines to reflect the new symbol/type/error behavior and
.d.tsemit changes.
Reviewed changes
Copilot reviewed 90 out of 90 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionImportedNames10.baseline.jsonc.diff | Baseline diff removed due to updated go-to-definition expectation. |
| testdata/baselines/reference/submodule/fourslash/goToDefinition/goToDefinitionImportedNames10.baseline.jsonc | Updates fourslash baseline markers/targets for definition resolution. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.types.diff | Updates expected type baseline diff for improved CJS export typing. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.types | Updates inferred types (e.g. property types) after alias export changes. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.symbols.diff | Updates expected symbol baseline diff for improved symbol binding. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.symbols | Updates symbol resolution for JSDoc/CJS exported class cases. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.errors.txt.diff | Updates expected diagnostic baseline diff after behavior changes. |
| testdata/baselines/reference/submodule/conformance/typeFromParamTagForFunction.errors.txt | Updates diagnostics (notably removing value-as-type error for B). |
| testdata/baselines/reference/submodule/conformance/nestedDestructuringOfRequire.js.diff | Updates .d.ts emit baseline diff for CJS export shape. |
| testdata/baselines/reference/submodule/conformance/nestedDestructuringOfRequire.js | Updates emitted declaration form (declare const + export {} pattern). |
| testdata/baselines/reference/submodule/conformance/moduleExportsAliasLoop2.symbols.diff | Updates symbol-baseline diff for alias-loop scenarios. |
| testdata/baselines/reference/submodule/conformance/moduleExportsAliasLoop2.symbols | Updates symbol resolutions in alias-loop cases. |
| testdata/baselines/reference/submodule/conformance/moduleExportsAliasLoop1.symbols.diff | Updates symbol-baseline diff for alias-loop scenarios. |
| testdata/baselines/reference/submodule/conformance/moduleExportsAliasLoop1.symbols | Updates symbol resolutions in alias-loop cases. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.types.diff | Updates type-baseline diff for nested namespace export scenarios. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.types | Updates type results for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.symbols.diff | Updates symbol-baseline diff for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.symbols | Updates symbol bindings for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.errors.txt.diff | Updates diagnostics baseline diff for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/moduleExportNestedNamespaces.errors.txt | Updates diagnostics after alias export behavior change. |
| testdata/baselines/reference/submodule/conformance/moduleExportAliasElementAccessExpression.js.diff | Updates .d.ts emit baseline diff for element-access export names. |
| testdata/baselines/reference/submodule/conformance/moduleExportAliasElementAccessExpression.js | Updates .d.ts output and associated TS18057 error locations. |
| testdata/baselines/reference/submodule/conformance/lateBoundAssignmentDeclarationSupport7.symbols.diff | Updates symbol-baseline diff for late-bound assignment cases. |
| testdata/baselines/reference/submodule/conformance/lateBoundAssignmentDeclarationSupport7.symbols | Updates symbol identity/qualification in late-bound assignment. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.types.diff | Baseline diff removed due to updated type output alignment. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.types | Updates type results around chained assignments on exports. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.symbols.diff | Updates symbol-baseline diff for chained assignment scenario. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.symbols | Updates symbol resolution for undefined in chained exports assignment. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.errors.txt.diff | Updates diagnostic baseline diff around many-member export assignment. |
| testdata/baselines/reference/submodule/conformance/jsdocTypeFromChainedAssignment3.errors.txt | Updates diagnostics/counts for member-based export typing. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.types.diff | Baseline diff removed due to improved typing for import('./mod1').C. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.types | Updates types to correctly resolve imported class alias as a type. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.symbols.diff | Updates symbol-baseline diff for imported type reference scenario. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.symbols | Updates symbol resolution for c.s against exported class alias. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.errors.txt.diff | Baseline diff removed after eliminating TS2694 error. |
| testdata/baselines/reference/submodule/conformance/jsdocImportTypeReferenceToClassAlias.errors.txt | Removes now-unexpected TS2694 diagnostic baseline content. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsImportAliasExposedWithinNamespaceCjs.js.diff | Updates .d.ts baseline diff for namespace/type export shape. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsImportAliasExposedWithinNamespaceCjs.js | Updates .d.ts output to include declare const myTypes plus type exports. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsFunctionsCjs.js.diff | Updates .d.ts baseline diff for function exports (export {} forms). |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsFunctionsCjs.js | Updates emitted d.ts exports from export declare var to export { ... }. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportSubAssignments(target=es2015).js.diff | Updates .d.ts baseline diff for secondary exports alongside export =. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportSubAssignments(target=es2015).js | Updates .d.ts output to declare const + export { Strings }. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportForms(target=es2015).js.diff | Updates .d.ts baseline diff for CJS export forms and error set. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportForms(target=es2015).js | Updates .d.ts output and expected TS2502 errors after export-shape change. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportAssignmentExpressionPlusSecondary(target=es2015).js.diff | Updates .d.ts baseline diff for export = + secondary export. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportAssignmentExpressionPlusSecondary(target=es2015).js | Updates .d.ts output to declare const Strings then export { Strings }. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportAssignedClassExpressionShadowing(target=es2015).js.diff | Updates .d.ts baseline diff for class-expression export aliasing. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsExportAssignedClassExpressionShadowing(target=es2015).js | Updates .d.ts to export { Q as Another }. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsClassStatic(target=es2015).js.diff | Updates .d.ts baseline diff for static + secondary export handling. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsClassStatic(target=es2015).js | Updates .d.ts output to declare const Strings + export { Strings }. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsClassExtendsVisibility(target=es2015).js.diff | Updates .d.ts baseline diff for extends + secondary export handling. |
| testdata/baselines/reference/submodule/conformance/jsDeclarationsClassExtendsVisibility(target=es2015).js | Updates .d.ts output to declare const Strings + export { Strings }. |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.types.diff | Baseline diff removed due to updated type/error expectations. |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.types | Updates type outputs for property-assignment name resolution case. |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.symbols.diff | Updates symbol-baseline diff for property-assignment name resolution. |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.symbols | Updates symbol output (notably for new D() case). |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.errors.txt.diff | Baseline diff removed due to diagnostic expectation change. |
| testdata/baselines/reference/submodule/conformance/exportPropertyAssignmentNameResolution.errors.txt | Updates diagnostic output for new D() name resolution. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.types.diff | Updates type-baseline diff for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.types | Updates type results for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.symbols.diff | Updates symbol-baseline diff for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.symbols | Updates symbol bindings for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.errors.txt.diff | Updates diagnostic baseline diff for nested namespace exports. |
| testdata/baselines/reference/submodule/conformance/exportNestedNamespaces.errors.txt | Updates diagnostics after alias export behavior change. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.types.diff | Baseline diff removed due to updated type output alignment. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.types | Updates types for exported class-expression import usage. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.symbols.diff | Updates symbol-baseline diff for exported class-expression import usage. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.symbols | Updates symbol resolution for k.values against exported class expression. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.js.diff | Updates .d.ts baseline diff; now includes DtsFileErrors output section. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.js | Updates .d.ts output for exported class expression (constructor return type now references K). |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.errors.txt.diff | Baseline diff removed after diagnostic expectation change. |
| testdata/baselines/reference/submodule/conformance/commonJSImportExportedClassExpression.errors.txt | Removes now-unexpected diagnostic baseline content. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.types.diff | Baseline diff removed due to updated type output alignment. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.types | Updates types for class type reference import scenario. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.symbols.diff | Updates symbol-baseline diff for class type reference import scenario. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.symbols | Updates symbol resolution for k.values against imported class symbol. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.js.diff | Updates .d.ts baseline diff to prefer export { K }. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.js | Updates .d.ts output to declare class K + export { K }. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.errors.txt.diff | Baseline diff removed after diagnostic expectation change. |
| testdata/baselines/reference/submodule/conformance/commonJSImportClassTypeReference.errors.txt | Removes now-unexpected diagnostic baseline content. |
| testdata/baselines/reference/submodule/conformance/commonJSAliasedExport.js.diff | Updates .d.ts baseline diff for CJS aliased export secondary member. |
| testdata/baselines/reference/submodule/conformance/commonJSAliasedExport.js | Updates .d.ts output to export { funky }. |
| testdata/baselines/reference/submodule/compiler/resolveNameWithNamspace.types.diff | Baseline diff removed due to updated type output alignment. |
| testdata/baselines/reference/submodule/compiler/resolveNameWithNamspace.types | Updates type results for exports.equal resolution in compiler baseline. |
| internal/transformers/declarations/transform.go | Adds export { source as name } emission for eligible CJS exports and helper isCommonJSAliasExport. |
| internal/checker/emitresolver.go | Treats exports.<prop> assignments as CommonJS exports for alias marking. |
| internal/checker/checker.go | Adjusts logic that ignores undefined assignments to repeated CJS export properties. |
| internal/binder/nameresolver.go | Adds CommonJS-specific filtering when resolving module exports (now flags-dependent). |
| internal/binder/binder.go | Marks eligible CJS export-property assignments as alias symbols at bind time. |
| internal/ast/utilities.go | Extends alias-symbol-declaration detection to cover exports.<symbol> = <alias>. |
| if ast.IsSourceFile(location) && location.AsSourceFile().CommonJSModuleIndicator != nil && result.Flags&ast.SymbolFlagsType == 0 { | ||
| result = nil |
There was a problem hiding this comment.
The CommonJS-specific filter here clears result whenever the found export has no SymbolFlagsType, even if the lookup meaning is for values. That will make value-only CommonJS exports (e.g. functions/vars) fail to resolve from moduleExports, which can break typing for patterns like exports.foo = ...; exports.foo().
Consider restricting this logic to type/namespace lookups only (e.g. only apply the filter when meaning includes type/namespace and the symbol lacks those meanings), rather than filtering all module-member lookups in CommonJS files.
| if ast.IsSourceFile(location) && location.AsSourceFile().CommonJSModuleIndicator != nil && result.Flags&ast.SymbolFlagsType == 0 { | |
| result = nil | |
| if ast.IsSourceFile(location) && location.AsSourceFile().CommonJSModuleIndicator != nil { | |
| requestedNonValueMeaning := meaning & ast.SymbolFlagsModuleMember &^ ast.SymbolFlagsValue | |
| if requestedNonValueMeaning != 0 && result.Flags&requestedNonValueMeaning == 0 { | |
| result = nil | |
| } else { | |
| break loop | |
| } |
There was a problem hiding this comment.
This is the intended behavior. A CJS module can't see it's own value exports (because they appear to be properties on the exports object). However, @typedef types are visible within the module.
There was a problem hiding this comment.
This does sort of tie in with the other comment, in that I think what this means is that exports.Foo = class {} will cause a type Foo to appear in scope?
There was a problem hiding this comment.
No, exports.Foo = class {} creates an alias named Foo, not a type. So it is not in scope inside the module.
| if ast.IsBinaryExpression(node) && ast.IsIdentifier(node.AsBinaryExpression().Right) { | ||
| if symbol := node.Symbol(); symbol != nil && len(symbol.Declarations) == 1 { | ||
| return true |
There was a problem hiding this comment.
isCommonJSAliasExport only treats exports.X = <identifier> as an alias export. But elsewhere (e.g. ast.ExpressionIsAlias) class expressions are also treated as aliases, and the PR description says class-expression exports should be supported. With the current restriction, exports.X = class ... will fall back to the export var X: ... path, which can yield self-referential/invalid .d.ts types (e.g. a constructor signature returning X when X is only a value).
Either extend alias-export handling to cover class expressions (by emitting a corresponding declare class + export) or explicitly fall back to a structural type to avoid emitting .d.ts that produces TS2749/TS2502 errors.
| if ast.IsBinaryExpression(node) && ast.IsIdentifier(node.AsBinaryExpression().Right) { | |
| if symbol := node.Symbol(); symbol != nil && len(symbol.Declarations) == 1 { | |
| return true | |
| if ast.IsBinaryExpression(node) { | |
| right := node.AsBinaryExpression().Right | |
| if ast.ExpressionIsAlias(right) { | |
| if symbol := node.Symbol(); symbol != nil && len(symbol.Declarations) == 1 { | |
| return true | |
| } |
There was a problem hiding this comment.
Agreed, there's more work to be done for class expressions. But doesn't need to be in this PR.
jakebailey
left a comment
There was a problem hiding this comment.
Seems okay for now given the edge cases already noted here
With this PR, we export alias symbols for CJS
exports.Foo = ...assignments where the expression is an entity name or a class expression. This is similar to our behavior in Strada and ensures type and/or namespace meanings are preserved for such exports. For exampleis now similar in behavior to
The PR makes a best effort to emit correct declaration files, but there are still issues when the exported entity is a qualified name or a class expression.