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

Core: Don't shadow the window global variable #14472

Merged
merged 3 commits into from
Apr 10, 2021

Conversation

eirslett
Copy link
Contributor

@eirslett eirslett commented Apr 4, 2021

Issue: Vite's hot module reloading doesn't work properly with the 'global' package

This snippet:

import { window } from 'global';

will let you mock window variables in Jest tests.
However, it breaks hot module reloading in Vite,
because Vite injects code snippets in top of every
file:

window.STUFF = 'Vite injected some HMR code here';
import { window } from 'global';
// this fails: Uncaught ReferenceError: Cannot access 'window' before initialization

What I did

The simple solution is to rename window when importing it:

import { window as globalWindow } from 'global';
globalWindow.alert('hello world');

The code works like before, but Vite will be able to
inject its code snippet without interfering with the
rest of the code.

How to test

Run Storybook CI, and check that nothing is broken.
I verified locally that this fixes the Vite HMR bug (but the Vite stuff is going on in branches in forks of Storybook, so it's hard for others to test).

This snippet:

```
import { window } from 'global';
```

will let you mock window variables in Jest tests.
However, it breaks hot module reloading in Vite,
because Vite injects code snippets in top of every
file:

```
window.STUFF = 'Vite injected some HMR code here';
import { window } from 'global';
// this fails: Uncaught ReferenceError: Cannot access 'window' before initialization
```

The simple solution is to rename `window` when importing it:

```
import { window as globalWindow } from 'global';
globalWindow.alert('hello world');
```

The code works like before, but Vite will be able to
inject its code snippet without interfering with the
rest of the code.
@shilman shilman changed the title Don't shadow the window global variable Core: Don't shadow the window global variable Apr 10, 2021
@shilman shilman added core maintenance User-facing maintenance tasks labels Apr 10, 2021
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ndelangen @tmeasday any objections?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Merging to unblock the vite work

@shilman shilman merged commit 3ec358f into storybookjs:next Apr 10, 2021
@eirslett eirslett deleted the dont-shadow-window branch April 12, 2021 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants