Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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
56 changes: 47 additions & 9 deletions internal/checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -5448,7 +5448,7 @@ func (c *Checker) checkExternalModuleExports(node *ast.Node) {
links := c.moduleSymbolLinks.Get(moduleSymbol)
if !links.exportsChecked {
exportEqualsSymbol := moduleSymbol.Exports[ast.InternalSymbolNameExportEquals]
if exportEqualsSymbol != nil && c.hasExportedMembers(moduleSymbol) {
if exportEqualsSymbol != nil && c.hasExportedMembers(moduleSymbol, exportEqualsSymbol.ValueDeclaration.Kind == ast.KindJSExportAssignment) {
declaration := core.OrElse(c.getDeclarationOfAliasSymbol(exportEqualsSymbol), exportEqualsSymbol.ValueDeclaration)
if declaration != nil && !isTopLevelInExternalModuleAugmentation(declaration) {
c.error(declaration, diagnostics.An_export_assignment_cannot_be_used_in_a_module_with_other_exported_elements)
Expand Down Expand Up @@ -5484,10 +5484,17 @@ func (c *Checker) checkExternalModuleExports(node *ast.Node) {
}
}

func (c *Checker) hasExportedMembers(moduleSymbol *ast.Symbol) bool {
func (c *Checker) hasExportedMembers(moduleSymbol *ast.Symbol, isCommonJS bool) bool {
for id := range moduleSymbol.Exports {
if id != ast.InternalSymbolNameExportEquals {
return true
if !isCommonJS {
return true
}
for _, declaration := range moduleSymbol.Exports[id].Declarations {
if declaration.Kind != ast.KindJSTypeAliasDeclaration {
return true
}
}
}
}
return false
Expand Down Expand Up @@ -13936,7 +13943,7 @@ func (c *Checker) getTargetOfImportEqualsDeclaration(node *ast.Node, dontResolve
moduleReference = ast.GetExternalModuleImportEqualsDeclarationExpression(node)
}
immediate := c.resolveExternalModuleName(node, moduleReference, false /*ignoreErrors*/)
resolved := c.resolveExternalModuleSymbol(immediate, false /*dontResolveAlias*/)
resolved := c.resolveExternalModuleSymbol(immediate, dontResolveAlias)
if resolved != nil && core.ModuleKindNode20 <= c.moduleKind && c.moduleKind <= core.ModuleKindNodeNext {
moduleExports := c.getExportOfModule(resolved, ast.InternalSymbolNameModuleExports, node, dontResolveAlias)
if moduleExports != nil {
Expand Down Expand Up @@ -15247,10 +15254,10 @@ func (c *Checker) resolveEntityName(name *ast.Node, meaning ast.SymbolFlags, ign
symbol = c.getMergedSymbol(c.resolveName(resolveLocation, name.AsIdentifier().Text, meaning, message, true /*isUse*/, false /*excludeGlobals*/))
case ast.KindQualifiedName:
qualified := name.AsQualifiedName()
symbol = c.resolveQualifiedName(name, qualified.Left, qualified.Right, meaning, ignoreErrors, dontResolveAlias, location)
symbol = c.resolveQualifiedName(name, qualified.Left, qualified.Right, meaning, ignoreErrors, location)
Copy link
Member Author

Choose a reason for hiding this comment

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

this is a driveby fix--dontResolveAlias wasn't used even before this PR

Copy link
Member

Choose a reason for hiding this comment

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

In Strada I see dontResolveAlias being given both true and false all over the place. Are you sure we should remove it?

Copy link
Member Author

@sandersn sandersn Nov 18, 2025

Choose a reason for hiding this comment

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

Yes. As best I can tell, it's a translation artifact--resolveQualifiedName is a function extracted from resolveEntityName -- it reads way better in Go and probably would in TS too -- but its code never actually refers to dontResolveAlias, even in Strada. It's only checked once in resolveEntityName, actually, right at the end, when deciding whether to resolveAlias on the symbol we just resolved.

case ast.KindPropertyAccessExpression:
access := name.AsPropertyAccessExpression()
symbol = c.resolveQualifiedName(name, access.Expression, access.Name(), meaning, ignoreErrors, dontResolveAlias, location)
symbol = c.resolveQualifiedName(name, access.Expression, access.Name(), meaning, ignoreErrors, location)
default:
panic("Unknown entity name kind")
}
Expand All @@ -15268,10 +15275,20 @@ func (c *Checker) resolveEntityName(name *ast.Node, meaning ast.SymbolFlags, ign
return symbol
}

func (c *Checker) resolveQualifiedName(name *ast.Node, left *ast.Node, right *ast.Node, meaning ast.SymbolFlags, ignoreErrors bool, dontResolveAlias bool, location *ast.Node) *ast.Symbol {
namespace := c.resolveEntityName(left, ast.SymbolFlagsNamespace, ignoreErrors, false /*dontResolveAlias*/, location)
func (c *Checker) resolveQualifiedName(name *ast.Node, left *ast.Node, right *ast.Node, meaning ast.SymbolFlags, ignoreErrors bool, location *ast.Node) *ast.Symbol {
namespace := c.resolveEntityName(left, ast.SymbolFlagsNamespace, true /*ignoreErrors*/, false /*dontResolveAlias*/, location)
if namespace == nil || ast.NodeIsMissing(right) {
return nil
alias := c.resolveEntityName(left, ast.SymbolFlagsAlias, true /*ignoreErrors*/, true /*dontResolveAlias*/, location)
if alias != nil && alias.Name == ast.InternalSymbolNameExportEquals {
// resolve typedefs exported from commonjs, stored on the module symbol
namespace = alias.Parent
}
if namespace == nil {
if !ignoreErrors {
c.resolveEntityName(left, ast.SymbolFlagsNamespace, ignoreErrors, false /*dontResolveAlias*/, location)
}
return nil
}
}
if namespace == c.unknownSymbol {
return namespace
Expand Down Expand Up @@ -15624,12 +15641,26 @@ func (c *Checker) getExportsOfModuleWorker(moduleSymbol *ast.Symbol) (exports as
}
return symbols
}
var originalModule *ast.Symbol
if moduleSymbol != nil {
if c.resolveSymbolEx(moduleSymbol.Exports[ast.InternalSymbolNameExportEquals], false /*dontResolveAlias*/) != nil {
originalModule = moduleSymbol
}
}
// A module defined by an 'export=' consists of one export that needs to be resolved
moduleSymbol = c.resolveExternalModuleSymbol(moduleSymbol, false /*dontResolveAlias*/)
exports = visit(moduleSymbol, nil, false)
if exports == nil {
exports = make(ast.SymbolTable)
}
// A CommonJS module defined by an 'export=' might also export typedefs, stored on the original module
if originalModule != nil && len(originalModule.Exports) > 1 {
for _, symbol := range originalModule.Exports {
if symbol.Flags&ast.SymbolFlagsType != 0 && symbol.Name != ast.InternalSymbolNameExportEquals {
exports[symbol.Name] = symbol
}
}
}
for name := range nonTypeOnlyNames.Keys() {
delete(typeOnlyExportStarMap, name)
}
Expand Down Expand Up @@ -23887,6 +23918,13 @@ func (c *Checker) getTypeFromImportTypeNode(node *ast.Node) *Type {
symbolFromVariable = c.getPropertyOfTypeEx(c.getTypeOfSymbol(mergedResolvedSymbol), current.Text(), false /*skipObjectFunctionPropertyAugment*/, true /*includeTypeOnlyMembers*/)
} else {
symbolFromModule = c.getSymbol(c.getExportsOfSymbol(mergedResolvedSymbol), current.Text(), meaning)
if symbolFromModule == nil {
// a CommonJS module might have typedefs exported alongside an export=
immediateModuleSymbol := c.resolveExternalModuleSymbol(innerModuleSymbol, true /*dontResolveAlias*/)
if immediateModuleSymbol != nil && core.Some(immediateModuleSymbol.Declarations, func(d *ast.Node) bool { return d.Kind == ast.KindJSExportAssignment }) {
symbolFromModule = c.getSymbol(c.getExportsOfSymbol(immediateModuleSymbol.Parent), current.Text(), meaning)
}
}
}
next := core.OrElse(symbolFromModule, symbolFromVariable)
if next == nil {
Expand Down
1 change: 0 additions & 1 deletion internal/fourslash/_scripts/failingTests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ TestIndirectClassInstantiation
TestInstanceTypesForGenericType1
TestJavascriptModules20
TestJavascriptModules21
TestJavascriptModulesTypeImport
TestJsDocAugments
TestJsDocExtends
TestJsDocFunctionSignatures10
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

func TestJavascriptModulesTypeImport(t *testing.T) {
t.Parallel()
t.Skip()

defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `// @allowJs: true
// @Filename: types.js
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use.js(1,22): error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.


==== typedefModuleExportsIndirect1.js (0 errors) ====
/** @typedef {{ a: 1, m: 1 }} C */
const dummy = 0;
module.exports = dummy;
==== use.js (1 errors) ====
/** @typedef {import('./controlFlowJSClassProperty').C} C */
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.
/** @type {C} */
var c

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect1.ts] ////

//// [typedefModuleExportsIndirect1.js]
/** @typedef {{ a: 1, m: 1 }} C */
const dummy = 0;
module.exports = dummy;
//// [use.js]
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c


//// [typedefModuleExportsIndirect1.js]
"use strict";
/** @typedef {{ a: 1, m: 1 }} C */
const dummy = 0;
module.exports = dummy;
//// [use.js]
"use strict";
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c;


//// [typedefModuleExportsIndirect1.d.ts]
export type C = {
a: 1;
m: 1;
};
export = dummy;
//// [use.d.ts]
type C = import('./controlFlowJSClassProperty').C;
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
declare var c: C;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect1.ts] ////

=== typedefModuleExportsIndirect1.js ===
/** @typedef {{ a: 1, m: 1 }} C */
const dummy = 0;
>dummy : Symbol(dummy, Decl(typedefModuleExportsIndirect1.js, 1, 5))

module.exports = dummy;
>module.exports : Symbol(dummy, Decl(typedefModuleExportsIndirect1.js, 1, 5))
>module : Symbol(module.exports)
>exports : Symbol(dummy, Decl(typedefModuleExportsIndirect1.js, 1, 5))
>dummy : Symbol(dummy, Decl(typedefModuleExportsIndirect1.js, 1, 5))

=== use.js ===
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c
>c : Symbol(c, Decl(use.js, 2, 3))

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect1.ts] ////

=== typedefModuleExportsIndirect1.js ===
/** @typedef {{ a: 1, m: 1 }} C */
const dummy = 0;
>dummy : 0
>0 : 0

module.exports = dummy;
>module.exports = dummy : 0
>module.exports : 0
>module : { readonly dummy: 0; }
>exports : 0
>dummy : 0

=== use.js ===
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c
>c : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use.js(1,22): error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.


==== typedefModuleExportsIndirect2.js (0 errors) ====
/** @typedef {{ a: 1, m: 1 }} C */
const f = function() {};
module.exports = f;
==== use.js (1 errors) ====
/** @typedef {import('./controlFlowJSClassProperty').C} C */
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.
/** @type {C} */
var c

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect2.ts] ////

//// [typedefModuleExportsIndirect2.js]
/** @typedef {{ a: 1, m: 1 }} C */
const f = function() {};
module.exports = f;
//// [use.js]
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c


//// [typedefModuleExportsIndirect2.js]
"use strict";
/** @typedef {{ a: 1, m: 1 }} C */
const f = function () { };
module.exports = f;
//// [use.js]
"use strict";
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c;


//// [typedefModuleExportsIndirect2.d.ts]
export type C = {
a: 1;
m: 1;
};
export = f;
//// [use.d.ts]
type C = import('./controlFlowJSClassProperty').C;
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
declare var c: C;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect2.ts] ////

=== typedefModuleExportsIndirect2.js ===
/** @typedef {{ a: 1, m: 1 }} C */
const f = function() {};
>f : Symbol(f, Decl(typedefModuleExportsIndirect2.js, 1, 5))

module.exports = f;
>module.exports : Symbol(f, Decl(typedefModuleExportsIndirect2.js, 1, 5))
>module : Symbol(module.exports)
>exports : Symbol(f, Decl(typedefModuleExportsIndirect2.js, 1, 5))
>f : Symbol(f, Decl(typedefModuleExportsIndirect2.js, 1, 5))

=== use.js ===
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c
>c : Symbol(c, Decl(use.js, 2, 3))

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect2.ts] ////

=== typedefModuleExportsIndirect2.js ===
/** @typedef {{ a: 1, m: 1 }} C */
const f = function() {};
>f : () => void
>function() {} : () => void

module.exports = f;
>module.exports = f : () => void
>module.exports : () => void
>module : { readonly f: () => void; }
>exports : () => void
>f : () => void

=== use.js ===
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c
>c : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use.js(1,22): error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.


==== typedefModuleExportsIndirect3.js (0 errors) ====
/** @typedef {{ a: 1, m: 1 }} C */
const o = {};
module.exports = o;
==== use.js (1 errors) ====
/** @typedef {import('./controlFlowJSClassProperty').C} C */
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2307: Cannot find module './controlFlowJSClassProperty' or its corresponding type declarations.
/** @type {C} */
var c

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect3.ts] ////

//// [typedefModuleExportsIndirect3.js]
/** @typedef {{ a: 1, m: 1 }} C */
const o = {};
module.exports = o;
//// [use.js]
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c


//// [typedefModuleExportsIndirect3.js]
"use strict";
/** @typedef {{ a: 1, m: 1 }} C */
const o = {};
module.exports = o;
//// [use.js]
"use strict";
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c;


//// [typedefModuleExportsIndirect3.d.ts]
export type C = {
a: 1;
m: 1;
};
export = o;
//// [use.d.ts]
type C = import('./controlFlowJSClassProperty').C;
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
declare var c: C;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/salsa/typedefModuleExportsIndirect3.ts] ////

=== typedefModuleExportsIndirect3.js ===
/** @typedef {{ a: 1, m: 1 }} C */
const o = {};
>o : Symbol(o, Decl(typedefModuleExportsIndirect3.js, 1, 5))

module.exports = o;
>module.exports : Symbol(o, Decl(typedefModuleExportsIndirect3.js, 1, 5))
>module : Symbol(module.exports)
>exports : Symbol(o, Decl(typedefModuleExportsIndirect3.js, 1, 5))
>o : Symbol(o, Decl(typedefModuleExportsIndirect3.js, 1, 5))

=== use.js ===
/** @typedef {import('./controlFlowJSClassProperty').C} C */
/** @type {C} */
var c
>c : Symbol(c, Decl(use.js, 2, 3))

Loading