-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
remove window variable #13367
Comments
The WPT tests require |
@andreubotella can you give an example? I was trying to grep through wpt to see how difficult this would be but couldn’t find any usages that would block our tests. |
It's true that most WPT tests that we run, even the |
Would it also make sense to remove the Arguably, using
... and that crashes in Deno on import. |
That should be fixed by actually specifying a |
Hi! Recently I sought to use React Testing Library to test my components in my Fresh project but was unable to get it to work (denoland/fresh#427). Part of the problem is that the |
I'm not sure I agree with this statement. A window indicates a window. A visual window i.e. a browser or browser-like environment. |
Just a datapoint from in an application - trying to get an existing application ported to remix.run and Deno has been difficult in part because, yeah - I'm seeing libraries that detect |
|
Once this is removed, for backwards compatibility we can recommend people do Actually, would need to be: import "data:text/javascript,globalThis.window = globalThis;"; |
This commit deprecates `window` global and adds deprecation notice on each use of `window`. We decided to proceed with removal of `window` global variable in Deno 2.0. There's a lot of code in the wild that uses pattern like this: ``` if (typeof window !== "undefined) { ... } ``` to check if the code is being run in browser. However, this check passes fine in Deno and most often libraries that do this check try to access some browser API that is not available in Deno, or use DOM APIs (which are also not available in Deno). This situation has occurred multiple times already and it's unfeasible to expect the whole ecosystem to migrate to new check (and even if that happened there's a ton of code that's already shipped and won't change). The migration is straightfoward - replace all usages of `window` with `globalThis` or `self`. When Deno encounters use of `window` global it will now issue a warning, steering users towards required changes: ``` Warning ├ Use of deprecated "window" API. │ ├ This API will be removed in Deno 2.0. Make sure to upgrade to a stable API before then. │ ├ Suggestion: Use `globalThis` or `self` instead. │ ├ Suggestion: You can provide `window` in the current scope with: `const window = globalThis`. │ └ Stack trace: └─ at file:///Users/ib/dev/deno/foo.js:7:1 ``` Ref #13367.
As of #25213 this is now in effect. |
The background for this is that many existing web frameworks incorrectly use
window
as a feature detect for web browsers. This breaks them when server side rendering in Deno, as they think they are running in a browser.The web frameworks should be feature detecting
document
if they want to know if they have access to the DOM, notwindow
.Nonetheless this practice is so prevalent in the ecosystem, that it would be difficult to change all of these wrong feature detects. This Sourcegraph query illustrates how widespread this is: https://sourcegraph.com/search?q=context:global+typeof+window&patternType=literal.
Because of this, we should consider removing the
window
global from Deno at the earliest possible date (likely Deno 2.0).The text was updated successfully, but these errors were encountered: