Use globalThis for .window/.self#715
Conversation
|
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. |
|
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. |
|
User tests look pretty good; there are lots of diffs in chrome-devtools, but they seem to be good. That project uses lots of |
Only those three? Currently we expose all members of PS: Okay, tried it locally and it's just that the tests doesn't cover other types. |
|
Turns out we don't need |
|
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 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 |
|
Quite a few errors after the new commit. I haven't looked at the reason yet. |
|
Eh. Removing // in cordova-plugin-x-socialsharing
interface Window {
plugins: CordovaPlugins;
}Looks like we should restore (Only jQuery error look related with |
|
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 |
|
Things like |
|
Okay, I see two options to solve the duplicated function overload problem (that is, only functions are problematic):
Would have been simpler if TS automatically merged them. |
|
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:
I'll check to see how the checker handles duplicate identical signatures. |
Yes, it is, so the duplication shouldn't cause any compiler error or unexpected compile time things. |
|
@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. |
|
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. |
|
Running tests now; I'll merge after the user tests and DT tests look good. |
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 DT run looks clean. A few ExpectType comments will need to change. |
2 participants
Should fix some DefinitelyTyped errors in #452 (comment).
Closes microsoft/TypeScript#19816