Skip to content

Skip ambient modules in globalThis #48938

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 5, 2022
Merged

Skip ambient modules in globalThis #48938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc39d91
Skip ambient modules in globalThis
sandersn May 3, 2022
ba8e45c
Skip only modules with every declaration ambient
sandersn May 4, 2022
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
2 changes: 1 addition & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11507,7 +11507,7 @@ namespace ts {
if (symbol === globalThisSymbol) {
const varsOnly = new Map<string, Symbol>() as SymbolTable;
members.forEach(p => {
if (!(p.flags & SymbolFlags.BlockScoped)) {
if (!(p.flags & SymbolFlags.BlockScoped) && !(p.flags & SymbolFlags.ValueModule && p.declarations?.length && every(p.declarations, isAmbientModule))) {
Copy link
Member

Choose a reason for hiding this comment

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

some questions:

  • what's this ValueModule symbol flag/what is a value module?
  • is it precise enough to check if some declaration is an ambient module? what if some of the other declarations are not ambient modules? (that question may be answered by the one above)

Copy link
Member Author

Choose a reason for hiding this comment

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

  • A value module is a namespace OR declare module "foo" with values inside it. A namespace module is a namespace that has only types inside it. (Both kinds of module can contain other namespaces; any nested value turns the parent namespaces into value modules.)
  • I think the binder only allows ambient modules to merge with ambient modules? I'll have to check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, the excludes are the same as other value modules: Value & ~(Function | Class | RegularEnum | ValueModule)

I'll see if I can name a function, class or enum "foo", but I don't think I can.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, effectively only other ambient modules can merge with ambient modules.

Copy link
Member

Choose a reason for hiding this comment

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

I'll see if I can name a function, class or enum "foo", but I don't think I can.

You can if you use a js merge, eg,

globalThis['"module"'] = function () {}

or similar.

Copy link
Member

Choose a reason for hiding this comment

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

some? What if you make a global named "module" that ?merges? with the ambient module named "module"? (Is such a merge even possible?)

Copy link
Member Author

@sandersn sandersn May 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

How do you make a global named "module"?

Edit: Ah, you give an example above. I guess every then?

varsOnly.set(p.escapedName, p);
}
});
Expand Down
21 changes: 21 additions & 0 deletions tests/baselines/reference/globalThisAmbientModules.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
tests/cases/conformance/es2019/globalThisAmbientModules.ts(8,39): error TS2339: Property '"ambientModule"' does not exist on type 'typeof globalThis'.
tests/cases/conformance/es2019/globalThisAmbientModules.ts(11,33): error TS2339: Property '"ambientModule"' does not exist on type 'typeof globalThis'.


==== tests/cases/conformance/es2019/globalThisAmbientModules.ts (2 errors) ====
declare module "ambientModule" {
export type typ = 1
export var val: typ
}
namespace valueModule { export var val = 1 }
namespace namespaceModule { export type typ = 1 }
// should error
type GlobalBad1 = (typeof globalThis)["\"ambientModule\""]
~~~~~~~~~~~~~~~~~~~
!!! error TS2339: Property '"ambientModule"' does not exist on type 'typeof globalThis'.
type GlobalOk1 = (typeof globalThis)["valueModule"]
type GlobalOk2 = globalThis.namespaceModule.typ
const bad1: (typeof globalThis)["\"ambientModule\""] = 'ambientModule'
~~~~~~~~~~~~~~~~~~~
!!! error TS2339: Property '"ambientModule"' does not exist on type 'typeof globalThis'.

20 changes: 20 additions & 0 deletions tests/baselines/reference/globalThisAmbientModules.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//// [globalThisAmbientModules.ts]
declare module "ambientModule" {
export type typ = 1
export var val: typ
}
namespace valueModule { export var val = 1 }
namespace namespaceModule { export type typ = 1 }
// should error
type GlobalBad1 = (typeof globalThis)["\"ambientModule\""]
type GlobalOk1 = (typeof globalThis)["valueModule"]
type GlobalOk2 = globalThis.namespaceModule.typ
const bad1: (typeof globalThis)["\"ambientModule\""] = 'ambientModule'


//// [globalThisAmbientModules.js]
var valueModule;
(function (valueModule) {
valueModule.val = 1;
})(valueModule || (valueModule = {}));
var bad1 = 'ambientModule';
38 changes: 38 additions & 0 deletions tests/baselines/reference/globalThisAmbientModules.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
=== tests/cases/conformance/es2019/globalThisAmbientModules.ts ===
declare module "ambientModule" {
>"ambientModule" : Symbol("ambientModule", Decl(globalThisAmbientModules.ts, 0, 0))

export type typ = 1
>typ : Symbol(typ, Decl(globalThisAmbientModules.ts, 0, 32))

export var val: typ
>val : Symbol(val, Decl(globalThisAmbientModules.ts, 2, 14))
>typ : Symbol(typ, Decl(globalThisAmbientModules.ts, 0, 32))
}
namespace valueModule { export var val = 1 }
>valueModule : Symbol(valueModule, Decl(globalThisAmbientModules.ts, 3, 1))
>val : Symbol(val, Decl(globalThisAmbientModules.ts, 4, 34))

namespace namespaceModule { export type typ = 1 }
>namespaceModule : Symbol(namespaceModule, Decl(globalThisAmbientModules.ts, 4, 44))
>typ : Symbol(typ, Decl(globalThisAmbientModules.ts, 5, 27))

// should error
type GlobalBad1 = (typeof globalThis)["\"ambientModule\""]
>GlobalBad1 : Symbol(GlobalBad1, Decl(globalThisAmbientModules.ts, 5, 49))
>globalThis : Symbol(globalThis)

type GlobalOk1 = (typeof globalThis)["valueModule"]
>GlobalOk1 : Symbol(GlobalOk1, Decl(globalThisAmbientModules.ts, 7, 58))
>globalThis : Symbol(globalThis)

type GlobalOk2 = globalThis.namespaceModule.typ
>GlobalOk2 : Symbol(GlobalOk2, Decl(globalThisAmbientModules.ts, 8, 51))
>globalThis : Symbol(globalThis)
>namespaceModule : Symbol(namespaceModule, Decl(globalThisAmbientModules.ts, 4, 44))
>typ : Symbol(namespaceModule.typ, Decl(globalThisAmbientModules.ts, 5, 27))

const bad1: (typeof globalThis)["\"ambientModule\""] = 'ambientModule'
>bad1 : Symbol(bad1, Decl(globalThisAmbientModules.ts, 10, 5))
>globalThis : Symbol(globalThis)

37 changes: 37 additions & 0 deletions tests/baselines/reference/globalThisAmbientModules.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
=== tests/cases/conformance/es2019/globalThisAmbientModules.ts ===
declare module "ambientModule" {
>"ambientModule" : typeof import("ambientModule")

export type typ = 1
>typ : 1

export var val: typ
>val : 1
}
namespace valueModule { export var val = 1 }
>valueModule : typeof valueModule
>val : number
>1 : 1

namespace namespaceModule { export type typ = 1 }
>typ : 1

// should error
type GlobalBad1 = (typeof globalThis)["\"ambientModule\""]
>GlobalBad1 : any
>globalThis : typeof globalThis

type GlobalOk1 = (typeof globalThis)["valueModule"]
>GlobalOk1 : typeof valueModule
>globalThis : typeof globalThis

type GlobalOk2 = globalThis.namespaceModule.typ
>GlobalOk2 : 1
>globalThis : any
>namespaceModule : any

const bad1: (typeof globalThis)["\"ambientModule\""] = 'ambientModule'
>bad1 : any
>globalThis : typeof globalThis
>'ambientModule' : "ambientModule"

11 changes: 11 additions & 0 deletions tests/cases/conformance/es2019/globalThisAmbientModules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
declare module "ambientModule" {
export type typ = 1
export var val: typ
}
namespace valueModule { export var val = 1 }
namespace namespaceModule { export type typ = 1 }
// should error
type GlobalBad1 = (typeof globalThis)["\"ambientModule\""]
type GlobalOk1 = (typeof globalThis)["valueModule"]
type GlobalOk2 = globalThis.namespaceModule.typ
const bad1: (typeof globalThis)["\"ambientModule\""] = 'ambientModule'