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

[fix] error on HMR with es6 bundles #5862

Merged
merged 2 commits into from Nov 27, 2017
Merged

Conversation

@Slashgear
Copy link
Contributor

@Slashgear Slashgear commented Oct 20, 2017

What kind of change does this PR introduce?

It's a bug fix.

Did you add tests for your changes?

Update test.

If relevant, link to documentation update:

N/A

Summary

We notice a bug in webpack HMR during our spike on creating es6 bundles.

Bug:

  • bootstrap dev bundle containes a this no longer bound to window, so it was undefined.

Fix:

  • Use window instead of this in both scripts templates.

This bug was identifed in #5776

Does this PR introduce a breaking change?

No.

Other information
Fix #5776
Made with @Osirisxxl

Fix 5776
@jsf-clabot
Copy link

@jsf-clabot jsf-clabot commented Oct 20, 2017

CLA assistant check
All committers have signed the CLA.

@webpack-bot
Copy link
Contributor

@webpack-bot webpack-bot commented Oct 20, 2017

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@sokra
sokra approved these changes Oct 22, 2017
Copy link
Member

@sokra sokra left a comment

Makes sense

Copy link
Contributor

@lukeapage lukeapage left a comment

I don’t think you can use window inside a web worker. Getting the global inside a es6 environment is difficult unless you know something about your environment.

@michael-ciniawsky
Copy link
Member

@michael-ciniawsky michael-ciniawsky commented Oct 24, 2017

Web Worker API

workers run in another global context that is different from the current window. This context is represented by a DedicatedWorkerGlobalScope object in the case of dedicated workers (standard workers that are utilized by a single script; shared workers use SharedWorkerGlobalScope).

You can run whatever code you like inside the worker thread, with some exceptions. For example, you can't directly manipulate the DOM from inside a worker, or use some default methods and properties of the window object.

WorkerGlobalScope.self ?

Copy link
Member

@sokra sokra left a comment

Should use self in WebWorker

@webpack-bot
Copy link
Contributor

@webpack-bot webpack-bot commented Oct 30, 2017

@Slashgear Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@sokra
sokra approved these changes Nov 27, 2017
@sokra sokra merged commit 801a253 into webpack:master Nov 27, 2017
9 of 12 checks passed
9 of 12 checks passed
codecov/changes/integration 1 file has unexpected coverage changes not visible in diff.
Details
codecov/project/integration 91.9% (-0.01%) compared to 0c4d69b
Details
coverage/coveralls Coverage decreased (-0.03%) to 93.841%
Details
ci/circleci Your tests passed on CircleCI!
Details
codacy/pr Good work! A positive pull request.
Details
codecov/changes/unit No unexpected coverage changes found.
Details
codecov/patch/integration Coverage not affected when comparing 0c4d69b...00daab2
Details
codecov/patch/unit Coverage not affected when comparing 0c4d69b...00daab2
Details
codecov/project/unit 49.96% (target 0%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@sokra
Copy link
Member

@sokra sokra commented Nov 27, 2017

Thanks

@liaobin312716
Copy link

@liaobin312716 liaobin312716 commented Dec 1, 2017

Hi, I got an error ReferenceError:Can't find variable:self(http://30.6.61.126:8081/...)

  • Webpack Version: 3.9.1
    But is works when webpack version is 3.8.1.

Then , I compared the compiled bundle file with the last .
version: 3.8.1
image
version: 3.9.1
image

Use self instead of this maybe bring some new problems ?

@jukben
Copy link

@jukben jukben commented Dec 2, 2017

Hi @sokra in https://github.com/callstack/haul we use target: webworker in general because Debug JS remotely in React Native Dev tools. So we experienced the same issue as @liaobin312716 when the JS running outside of webworker (self is not defined in iOS/Andorid JS env). When developer enforce "webworker mode" (code running in webworker) for remotely debug everything is fine. Is there anything that we can do about it? Maybe there is some setting how to mock "self" if it' not present in the current environment. var self = self || window would be sufficient. 🙈

We can't change mode on demand, that would result in restarting of Webpack which is time-consuming.

@dor-itzhaki

This comment has been minimized.

Copy link

@dor-itzhaki dor-itzhaki commented on lib/JsonpMainTemplatePlugin.js in 27e4d14 Dec 12, 2017

shouldn't that be "self" as done here: 00daab2 ?

@robaweb

This comment has been minimized.

After this commit building of WebWorkers as others entries (webpack.entry) does not work any more in dev-mode with hot reload. Actually builds goes success but in browser i see this error 'Uncaught ReferenceError: window is not defined'
Is it possible to change 'window' to 'self' ? I try it on my local machine and looks like it's work.

This comment has been minimized.

Copy link

@robaweb robaweb replied Jan 30, 2018

This comment has been minimized.

Copy link

@pedroteixeira pedroteixeira replied Feb 15, 2018

Are there any workarounds for this? Is this something fixed in v4?

This comment has been minimized.

Copy link

@pedroteixeira pedroteixeira replied Feb 15, 2018

as workaround, is there any plugin to allow a replace on the generated file to fix this?

This comment has been minimized.

Copy link

@chenxeed chenxeed replied Mar 20, 2018

I faced issue too on this, wish it could be fixed soon in new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet