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

Unable to extend window in TS 3.6 #33128

Closed
danvk opened this issue Aug 29, 2019 · 20 comments
Closed

Unable to extend window in TS 3.6 #33128

danvk opened this issue Aug 29, 2019 · 20 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@danvk
Copy link
Contributor

danvk commented Aug 29, 2019

TypeScript Version: 3.6.2

Search Terms:

  • globalThis

Code

This code passed the type checker in TS 3.5:

interface MyWindow extends Window {
  foo: string;
}

(window as MyWindow).foo = 'bar';

but fails with this error in TS 3.6:

Conversion of type 'Window & typeof globalThis' to type 'MyWindow' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Property 'foo' is missing in type 'Window & typeof globalThis' but required in type 'MyWindow'.ts(2352)

Following the release notes, I can work around the issue by changing from interface to type and using an intersection:

type MyWindow = (typeof window) & {
  foo: string;
}

(window as MyWindow).foo = 'bar';

but this feels more obscure than the old way. Is this WAI? What's the rationale for the non-extendable Window?

Expected behavior:

An interface should be able to extend Window.

Actual behavior:

The error above.

Playground Link: 3.6 isn't on the playground yet.

Related Issues:

@danvk
Copy link
Contributor Author

danvk commented Aug 29, 2019

Perhaps unrelated, but I ran across this spectacularly weird error while trying to work around this:

type T = Window & typeof globalThis;
interface MyWindow extends T {
  foo: string;
}

Error:

interface MyWindow
Property 'Infinity' of type 'number' is not assignable to numeric index type 'Window'.ts(2412)
Property 'NaN' of type 'number' is not assignable to numeric index type 'Window'.ts(2412)

@dragomirtitian
Copy link
Contributor

Augmentation of window is usually done by using interface merging:

export {}
declare global {
  interface Window {
    foo: string;
  }
}

window.foo = 'bar';

or

interface Window {
  foo: string;
}

window.foo = 'bar';

For non-module scenarios.

@danvk
Copy link
Contributor Author

danvk commented Aug 29, 2019

The appeal of extending Window and using an assertion is that it's not global. Though I suppose if you're extending a global singleton, you don't have much ground to stand on!

@fatcerberus
Copy link

When using modules, the interface merge will be local to the file it occurs in, I believe. It's true you can't make the scope any narrower than that, but at least it won't poison outside code.

@sandersn
Copy link
Member

sandersn commented Sep 3, 2019

Why not include globalThis at the point of the assertion?

(window as MyWindow & typeof globalThis).foo = 'bar';

This is indeed working as intended. window.foo = 'bar''s runtime behaviour is best represented by an interface merge, not a subtype. Can you actually even extend window? It seems like a convenient lie to the compiler.

As for the weird error, Window has a numeric index signature { [n: number]: Window } so that window[0] : Window. But when making sure an extends is legal, the compiler checks that numeric properties from the other side of the intersection have type Window. Normally, that's properties with names like '0' and '1', but guess what NaN and Infinity are also numbers, so we check that their types are compatible with Window's index signatures. Since NaN: number and number is not assignable to Window, the compiler reports that as an error.

@sandersn sandersn added the Working as Intended The behavior described is the intended behavior; this is not a bug label Sep 3, 2019
@danvk
Copy link
Contributor Author

danvk commented Sep 3, 2019

Thanks @sandersn. The missing piece was that window[0] is a way of accessing a frame on the page.

@danvk danvk closed this as completed Sep 3, 2019
@fatcerberus
Copy link

fatcerberus commented Sep 3, 2019

Normally, that's properties with names like '0' and '1', but guess what NaN and Infinity are also numbers, so we check that their types are compatible with Window's index signatures. Since NaN: number and number is not assignable to Window, the compiler reports that as an error.

Wait, what.

The index signature [n: number]: Window doesn't match "NaN": number or "Infinity": number at all (it's not a string index signature) so their types shouldn't matter for that purpose.

@sandersn
Copy link
Member

sandersn commented Sep 4, 2019

@fatcerberus the property is named NaN, which is definitely a number. The funniest number.

@fatcerberus
Copy link

Oh I see, I didn’t realize those were covered by a numeric index signature because they’re not valid array indices. I know they are covered by number but for some reason I thought the signature was special-cased to only cover valid array indices (i.e. roughly the domain of Number.isSafeInteger).

@danvk
Copy link
Contributor Author

danvk commented Sep 18, 2019

@fatcerberus I don't believe this statement is correct:

When using modules, the interface merge will be local to the file it occurs in, I believe. It's true you can't make the scope any narrower than that, but at least it won't poison outside code.

I put this in a new file in my project (foo.ts) that's not imported anywhere:

export {};
declare global {
  interface Document {
    /** documentation on foo */
    foo: string;
  }
}

Then, in another file, I can write:

document.foo;

without importing foo. There's no error and I see the documentation. So I do not believe the augmentation is scoped to the module at all. It is as global as the declare global implies.

wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-Wikibase that referenced this issue Sep 27, 2019
Typescript 3.6 causes an incompatibility in eslint (or at least the risk
of it) which makes for quite verbose warnings.
Cf. Ie0bfd9871384c4daf9a3d4a8e5801edcc0a27d5b

Also some of the challenges seen in I65d50c908ca706105f7d5e5b709487df7c908621
(having to cast a window as any before being able to make it a MwWindow)
go away on the lower version, which could be helpful until we follow the
suggested mitigation[0].

[0] microsoft/TypeScript#33128

Change-Id: Ic6e13f75a71b0212f81ebacd16d344fc8ea875e4
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions that referenced this issue Sep 27, 2019
* Update Wikibase from branch 'master'
  to 0f1c1752140e77f6c90fcc21271178a5d9bf9358
  - Merge "tainted ref: don't use TS 3.6 just yet"
  - tainted ref: don't use TS 3.6 just yet
    
    Typescript 3.6 causes an incompatibility in eslint (or at least the risk
    of it) which makes for quite verbose warnings.
    Cf. Ie0bfd9871384c4daf9a3d4a8e5801edcc0a27d5b
    
    Also some of the challenges seen in I65d50c908ca706105f7d5e5b709487df7c908621
    (having to cast a window as any before being able to make it a MwWindow)
    go away on the lower version, which could be helpful until we follow the
    suggested mitigation[0].
    
    [0] microsoft/TypeScript#33128
    
    Change-Id: Ic6e13f75a71b0212f81ebacd16d344fc8ea875e4
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-Wikibase that referenced this issue Jan 24, 2020
Apply latest TypeScript version[0] and fix resulting complaints.
The question of how to extend window may be noteworthy[1] as the
`declare global` in MwWindow does something profound but in a
well-hidden spot. I guess this should be made more prominent, maybe
in a @types/global.d.ts - acknowledging what's going on in practice.

eslint warnings about incompatibility with latest TS, as seen earlier
(b40683d), seem to no longer happen.

Let's leave applying goodies (e.g. "Optional Chaining") for later to
reduce the rebase effort for a dependent patch
(I8dea58b3b4c40b265d86b28b63cc8d692ac7929c).

[0] https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html
[1] microsoft/TypeScript#33128

Change-Id: I0d7f6228276f60eb4b3866f470311f59aeef5d39
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions that referenced this issue Jan 24, 2020
* Update Wikibase from branch 'master'
  to a5e6df0da5fa9b4cd9af63cb187a8477dc289775
  - Merge "bridge: update to TypeScript 3.7"
  - bridge: update to TypeScript 3.7
    
    Apply latest TypeScript version[0] and fix resulting complaints.
    The question of how to extend window may be noteworthy[1] as the
    `declare global` in MwWindow does something profound but in a
    well-hidden spot. I guess this should be made more prominent, maybe
    in a @types/global.d.ts - acknowledging what's going on in practice.
    
    eslint warnings about incompatibility with latest TS, as seen earlier
    (b40683d), seem to no longer happen.
    
    Let's leave applying goodies (e.g. "Optional Chaining") for later to
    reduce the rebase effort for a dependent patch
    (I8dea58b3b4c40b265d86b28b63cc8d692ac7929c).
    
    [0] https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html
    [1] microsoft/TypeScript#33128
    
    Change-Id: I0d7f6228276f60eb4b3866f470311f59aeef5d39
@kaminskypavel
Copy link

kaminskypavel commented Aug 17, 2020

quick and dirty 💥

(window as any).foo = "bar";

@Magiccwl
Copy link

Magiccwl commented Dec 21, 2020

Augmentation of window is usually done by using interface merging:

export {}
declare global {
  interface Window {
    foo: string;
  }
}

window.foo = 'bar';

or

interface Window {
  foo: string;
}

window.foo = 'bar';

For non-module scenarios.

Thx your answer, Im just curious whats the first line

export {}

for?

@orta
Copy link
Contributor

orta commented Dec 21, 2020

To force the file to be registered as a module, not a script

@leepowelldev
Copy link

To force the file to be registered as a module, not a script

@orta Can I ask why it needs to be registered as a module?

For me this doesn't work until the add the export {} - I'm just trying to understand why.

declare global {
  interface Window {
    myProp: string;
  }
}

@shicks
Copy link
Contributor

shicks commented Aug 23, 2021

@danvk wrote:

@fatcerberus I don't believe this statement is correct:

When using modules, the interface merge will be local to the file it occurs in, I believe. It's true you can't make the scope any narrower than that, but at least it won't poison outside code.

I put this in a new file in my project (foo.ts) that's not imported anywhere: declare global { interface Foo { foo: string; } }
Then, in another file, I can write: document.foo without importing foo. There's no error and I see the documentation. So I do not believe the augmentation is scoped to the module at all. It is as global as the declare global implies.

Super-late response here, but there exist some TypeScript wrappers that will treat this differently. In particular, at Google we redefine declare to mean "emit an externs file for all the world to see", mainly for the purpose of allowing folks to write declare interface in their modules to force property names to be externed and thus not mangled by the optimizer. Unfortunately, this had the unintended consequence that other legitimate uses of declare (e.g. locally-extending interfaces, or declare const PRIVATE: unique symbol to get a module-local type typeof PRIVATE) end up having globally-visible impact.

@erikeckhardt
Copy link

erikeckhardt commented Dec 13, 2021

If you want the type augmentation to be global and ambient, this works fine:

// somefile.d.ts
declare interface Window {
  foo: string
}

The trick is, you must put this in a file with the extension .d.ts (and there must not be a same-named file with just the extension .ts in the same directory). Then this will function as an ambient type and declaration merging will kick in. No import required.

@shicks
Copy link
Contributor

shicks commented Dec 13, 2021

The solution we recommend at Google is to make a local subtype for explicit casts, which avoids polluting the global type definitions. Because of the NaN/Infinity issue, we have a library with

type GlobalThis = typeof globalThis & Window & {
  NaN: never;
  Infinity: never;
};

which can then be used like so: (playground)

interface MyGlobal extends GlobalThis {
  readonly foo: number;
}
use((window as MyGlobal).foo);

@erikeckhardt
Copy link

@shicks Thanks for the tip! Makes sense. Some projects/packages are small enough and use of a global object pervasive enough that boosting a type to be globally ambient is useful (especially in packages that aren't consumed by other packages). But when polluting the global scope is harmful, this seems like a good technique to reduce the boilerplate and complexity of strongly typing at use-time.

@lizyChy0329
Copy link

If you want the type augmentation to be global and ambient, this works fine:

// somefile.d.ts
declare interface Window {
  foo: string
}

The trick is, you must put this in a file with the extension .d.ts (and there must not be a same-named file with just the extension .ts in the same directory. Then this will function as an ambient type and declaration merging will kick in. No import required.

This is correct method for window type extend

@louwers
Copy link

louwers commented Dec 20, 2024

Is it possible to extend Window with type? I want to use a mapped type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests