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

Add feature to disable hotReloading with query string #1068

Merged
merged 2 commits into from Sep 12, 2017

Conversation

edi9999
Copy link
Contributor

@edi9999 edi9999 commented Sep 4, 2017

What kind of change does this PR introduce?
Feature

Did you add or update the examples/?
No, but you could use any sample and use the queryParam : ?disableAutoreload=true to

Summary

See corresponding issue : #936

This option can be used if you want to use the same platform for developping and to run automated tests (the tests should not be impacted by a code change whenever they are running).

Does this PR introduce a breaking change?

No

Other information

@jsf-clabot
Copy link

jsf-clabot commented Sep 4, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Sep 4, 2017

Codecov Report

Merging #1068 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1068   +/-   ##
=======================================
  Coverage   73.17%   73.17%           
=======================================
  Files           5        5           
  Lines         451      451           
  Branches      143      143           
=======================================
  Hits          330      330           
  Misses        121      121

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f166177...234375b. Read the comment docs.

@edi9999
Copy link
Contributor Author

edi9999 commented Sep 8, 2017

Any updates ?
ping @shellscape

@shellscape
Copy link
Contributor

@edi9999 please do us a solid and wait about a week before pings 😄 I promise we see all activity here and will respond/comment as soon as possible. I'm currently preparing for hurricane irma and will be returning next week.

@edi9999
Copy link
Contributor Author

edi9999 commented Sep 8, 2017

Ok ! I hope everything will be OK for you

client/index.js Outdated
@@ -22,6 +22,10 @@ function getCurrentScriptSource() {
}

let urlParts;
let hotReloading = true;
if (typeof window !== 'undefined') {
hotReloading = window.location.search.indexOf('disableAutoReload') === -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

not a fan of negative-options, meaning a positive value will result in the negative reassignment of another variable. let's also keep the variable names the same as the querystring parameter. so;

let hotReload = true;
...
const qs = window.location.search.toLowerCase();
hotReload = !qs.indexOf('hotreload=false');

the code was trimmed for brevity, but those are the changes I'd like to see

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed :-)

@shellscape shellscape merged commit e519cf2 into webpack:master Sep 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants