Use globalThis for .window/.self #715

saschanaz · 2019-06-12T04:46:34Z

Should fix some DefinitelyTyped errors in #452 (comment).

Closes microsoft/TypeScript#19816

sandersn · 2019-06-12T20:44:14Z

I'll need to test this on our tests and on Definitely Typed as well, but it's a good change.

Let's hold #452 until this PR is in.

sandersn · 2019-06-12T21:58:19Z

Errors from Typescript tests: without #452, cancelAnimationFrame, setTimeout and fetch are duplicated. We'll need to make sure that this is not the case after both PRs are in.

I'm running user tests right now.

sandersn · 2019-06-12T22:32:03Z

User tests look pretty good; there are lots of diffs in chrome-devtools, but they seem to be good. That project uses lots of window.x references to refer to globals.

saschanaz · 2019-06-13T02:56:32Z

Errors from Typescript tests: without #452, cancelAnimationFrame, setTimeout and fetch are duplicated.

Only those three? Currently we expose all members of Window to global (to allow both window.performance and performance) so Window & typeof globalThis can have duplicated members, but this way other members should get the same errors too.

PS: Okay, tried it locally and it's just that the tests doesn't cover other types.

saschanaz · 2019-06-13T03:52:34Z

Turns out we don't need Window & part because we already add all the members to globalThis.

sandersn · 2019-06-13T17:02:12Z

That should take care of the duplicates. Here are the errors from definitely typed, although I'll re-run with your new changes.

I'm not sure why dojo runs out of memory. It doesn't when I just run tsc so I guess it's a problem with dtslint-runner.

Error in apollo-upload-client
Error: Errors in typescript@local for external dependencies:
node_modules/apollo-link-http-common/lib/index.d.ts(50,13): error TS2304: Cannot find name 'GlobalFetch'.

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)


Error in dom-inputevent
Error: Errors in typescript@local for external dependencies:
../../../ts/built/local/lib.dom.d.ts(581,5): error TS2687: All declarations of 'isComposing' must have identical modifiers.
../../../ts/built/local/lib.dom.d.ts(9656,11): error TS2300: Duplicate identifier 'InputEvent'.
../../../ts/built/local/lib.dom.d.ts(9662,13): error TS2300: Duplicate identifier 'InputEvent'.

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)


Error in dojo
Out of memory


Error in jquery
Error: /home/nathansa/DefinitelyTyped/types/jquery/jquery-tests.ts:195:13
ERROR: 195:13  expect  Expected type to be:
  JQuery<Window>
got:
  JQuery<Window & typeof globalThis>

/home/nathansa/DefinitelyTyped/types/jquery/test/jquery-slim-window-module-tests.ts:5:1
ERROR: 5:1     expect  Expected type to be:
  JQuery<Window>
got:
  JQuery<Window & typeof globalThis>

/home/nathansa/DefinitelyTyped/types/jquery/test/jquery-window-module-tests.ts:5:1
ERROR: 5:1     expect  Expected type to be:
  JQuery<Window>
got:
  JQuery<Window & typeof globalThis>

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)


Error in redux-storage
Error: /home/nathansa/DefinitelyTyped/types/redux-storage/redux-storage-tests.ts:26:63
ERROR: 26:63  expect  TypeScript@local compile error: 
Argument of type 'Reducer<TestState>' is not assignable to parameter of type 'Reducer<unknown>'.
  Types of parameters 'state' and 'state' are incompatible.
    Type 'unknown' is not assignable to type 'TestState'.
ERROR: 36:85  expect  TypeScript@local compile error: 
Argument of type 'Reducer<TestState>' is not assignable to parameter of type 'Reducer<unknown>'.

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)


Error in webappsec-credential-management
Error: Errors in typescript@local for external dependencies:
../../../ts/built/local/lib.dom.d.ts(3622,11): error TS2300: Duplicate identifier 'Credential'.
../../../ts/built/local/lib.dom.d.ts(3627,13): error TS2300: Duplicate identifier 'Credential'.
../../../ts/built/local/lib.dom.d.ts(10725,14): error TS2687: All declarations of 'credentials' must have identical modifiers.
../../../ts/built/local/lib.dom.d.ts(19853,6): error TS2300: Duplicate identifier 'CredentialMediationRequirement'.

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)


Error in webgl2
Error: /home/nathansa/DefinitelyTyped/types/webgl2/index.d.ts:582:13
ERROR: 582:13  expect  TypeScript@local compile error: 
Subsequent variable declarations must have the same type.  Variable 'WebGL2RenderingContext' must be of type '{ new (): WebGL2RenderingContext; prototype: WebGL2RenderingContext; readonly ACTIVE_ATTRIBUTES: number; readonly ACTIVE_TEXTURE: number; readonly ACTIVE_UNIFORMS: number; readonly ALIASED_LINE_WIDTH_RANGE: number; ... 554 more ...; readonly WAIT_FAILED: number; }', but here has type '{ new (): WebGL2RenderingContext; prototype: WebGL2RenderingContext; readonly ACTIVE_ATTRIBUTES: number; readonly ACTIVE_TEXTURE: number; readonly ACTIVE_UNIFORMS: number; readonly ALIASED_LINE_WIDTH_RANGE: number; ... 555 more ...; readonly MAX_CLIENT_WAIT_TIMEOUT_WEBGL: number; }'.

    at /home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/nathansa/dtslint-runner/node_modules/dtslint/bin/index.js:5:58)
The following packages had errors: apollo-upload-client, dom-inputevent, dojo, jquery, redux-storage, webappsec-credential-management, webgl2
FAILED

Note that we haven't updated DT for all the DOM changes of the last two months, so it's possible that these failures are not even related to window (webgl2 especially)

sandersn · 2019-06-13T18:15:56Z

Quite a few errors after the new commit. I haven't looked at the reason yet.
errors.txt

saschanaz · 2019-06-14T03:34:00Z

Eh. Removing Window & part will break augmentation to Window interface:

// in cordova-plugin-x-socialsharing
interface Window {
    plugins: CordovaPlugins;
}

Looks like we should restore Window & (and just deal with the duplicates), thoughts? (~~Alternatively we just could fix them with declare global, though.~~ Doesn't work, with TS2669. Maybe it should be just var plugins.)

(Only jQuery error look related with window in #715 (comment))

sandersn · 2019-06-14T16:47:40Z

Hm. I think Window augmentation is a well-established pattern, unfortunately. We should address the duplication instead.

Where does the duplication come from, globals that are also members of Window? I assume that fetch, performance et al can be removed from Window if so.

saschanaz · 2019-06-16T07:15:22Z

Things like var x: Window["fetch"] will fail then, but probably worth trying.

saschanaz · 2019-06-16T10:54:39Z

Okay, I see two options to solve the duplicated function overload problem (that is, only functions are problematic):

window: Omit<Window, keyof typeof globalThis> & typeof globalThis. The type becomes dirty mainly because of keyof.
Empty Window: interface Window {};. Will break Window["fetch"].

Would have been simpler if TS automatically merged them.

sandersn · 2019-06-18T16:10:53Z

The signature of fetch is generated from the same signature in the widl, isn't it? That makes me think there are two more options:

keep the duplication, since the signatures will be identical
change the emitter to not emit signatures in Window that would otherwise be duplicated.

I'll check to see how the checker handles duplicate identical signatures.

saschanaz · 2019-06-18T16:15:25Z

The signature of fetch is generated from the same signature in the widl

Yes, it is, so the duplication shouldn't cause any compiler error or unexpected compile time things.

sandersn · 2019-06-18T16:16:00Z

@weswigham does the checker have special handling for multiple signatures arising from an intersection? I thought there was, but I can't find it. As-is, it seems like an intersection with duplicate signatures will take the overload code path everywhere, but should behave like a single intersection in relationship checking, resolveCall, etc.

sandersn · 2019-07-09T15:30:11Z

Update: the checker now has special handling for multiple signatures in an intersection; they are deduped if they are identical. In this case they should be, so I think duplication is the best option.

sandersn · 2019-07-10T18:05:31Z

Running tests now; I'll merge after the user tests and DT tests look good.

sandersn · 2019-07-10T20:20:47Z

lodash, async, debug: One or two bad errors during feature detection are now gone.
chrome-devtools: weird name resolution pattern needs type annotation now:

var o = self
for (const n of names) {
  o = o[n]
}

chrome-devtools uses window property accesses to access globals, so this PR improves it quitea
bit.

DT run looks clean. A few ExpectType comments will need to change.

saschanaz force-pushed the globalthis branch from 699712c to 1389855 Compare June 12, 2019 04:49

Use globalThis for .window/.self

06be1c7

saschanaz force-pushed the globalthis branch from 51660e6 to 06be1c7 Compare July 10, 2019 01:41

sandersn mentioned this pull request Jul 10, 2019

Update @types/node and typescript dependency #737

Merged

sandersn approved these changes Jul 10, 2019

View reviewed changes

sandersn merged commit f96dc0a into microsoft:master Jul 10, 2019

saschanaz deleted the globalthis branch July 11, 2019 02:28

sandersn mentioned this pull request Aug 7, 2019

Window props Blob and URL do not exist in v3.6 beta/dev microsoft/TypeScript#32750

Closed

eps1lon mentioned this pull request Jan 27, 2020

fix(dom): intersect defaultView with globalThis #820

Merged

sandersn mentioned this pull request Aug 19, 2021

Intersection between separate types with getter and setter does not allow assignment to property microsoft/TypeScript#45122

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use globalThis for .window/.self #715

Use globalThis for .window/.self #715

saschanaz commented Jun 12, 2019 •

edited

sandersn commented Jun 12, 2019

sandersn commented Jun 12, 2019

sandersn commented Jun 12, 2019

saschanaz commented Jun 13, 2019 •

edited

saschanaz commented Jun 13, 2019

sandersn commented Jun 13, 2019

sandersn commented Jun 13, 2019

saschanaz commented Jun 14, 2019 •

edited

sandersn commented Jun 14, 2019

saschanaz commented Jun 16, 2019 •

edited

saschanaz commented Jun 16, 2019 •

edited

sandersn commented Jun 18, 2019

saschanaz commented Jun 18, 2019 •

edited

sandersn commented Jun 18, 2019

sandersn commented Jul 9, 2019

sandersn commented Jul 10, 2019

sandersn commented Jul 10, 2019

Use globalThis for .window/.self #715

Use globalThis for .window/.self #715

Conversation

saschanaz commented Jun 12, 2019 • edited

sandersn commented Jun 12, 2019

sandersn commented Jun 12, 2019

sandersn commented Jun 12, 2019

saschanaz commented Jun 13, 2019 • edited

saschanaz commented Jun 13, 2019

sandersn commented Jun 13, 2019

sandersn commented Jun 13, 2019

saschanaz commented Jun 14, 2019 • edited

sandersn commented Jun 14, 2019

saschanaz commented Jun 16, 2019 • edited

saschanaz commented Jun 16, 2019 • edited

sandersn commented Jun 18, 2019

saschanaz commented Jun 18, 2019 • edited

sandersn commented Jun 18, 2019

sandersn commented Jul 9, 2019

sandersn commented Jul 10, 2019

sandersn commented Jul 10, 2019

saschanaz commented Jun 12, 2019 •

edited

saschanaz commented Jun 13, 2019 •

edited

saschanaz commented Jun 14, 2019 •

edited

saschanaz commented Jun 16, 2019 •

edited

saschanaz commented Jun 16, 2019 •

edited

saschanaz commented Jun 18, 2019 •

edited