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

no babel-loader node_modules exclude/web_modules Fixes #1278 #1279

Closed
wants to merge 1 commit into from

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Jan 16, 2018

and babel@7 so it's safe to do so.

Fixes #1278

also remove ancient unused version of jquery hiding in web_modules.

@jsf-clabot
Copy link

jsf-clabot commented Jan 16, 2018

CLA assistant check
All committers have signed the CLA.

@graingert graingert changed the title no node_modules exclude no node_modules exclude Fixes #1278 Jan 16, 2018
@graingert graingert changed the title no node_modules exclude Fixes #1278 no babel-loader node_modules exclude Fixes #1278 Jan 16, 2018
and babel@7 so it's safe to do so.

also remove web_modules
@graingert graingert changed the title no babel-loader node_modules exclude Fixes #1278 no babel-loader node_modules exclude/web_modules Fixes #1278 Jan 16, 2018
@graingert
Copy link
Contributor Author

I had @nobitagit try this out in IE11 and it works great.

@graingert
Copy link
Contributor Author

thank you @sindresorhus for dragging us into the future!

@codecov
Copy link

codecov bot commented Jan 17, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1279   +/-   ##
=======================================
  Coverage   75.72%   75.72%           
=======================================
  Files           5        5           
  Lines         482      482           
  Branches      156      156           
=======================================
  Hits          365      365           
  Misses        117      117

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 8c1ed7a...377b146. Read the comment docs.

@graingert
Copy link
Contributor Author

graingert commented Jan 18, 2018

(replying from #1273 (comment))

@julianxhokaxhiu are you sure that's not in your own code?

Here's the bundled JS https://unpkg.com/@procensus/webpack-dev-server@2.11.0/client/index.bundle.js

@julianxhokaxhiu
Copy link

julianxhokaxhiu commented Jan 18, 2018

Yes definitely sure. Feel free to check the project here: https://github.com/julianxhokaxhiu/polysticky.js

//EDIT:
Found also the same issue here: vercel/next.js#2747

It seems that downgrading strip-ansi is the solution: https://github.com/zeit/next.js/pull/2860/commits

@graingert
Copy link
Contributor Author

graingert commented Jan 18, 2018

@julianxhokaxhiu you've not switched to @procensus/webpack-dev-server in polysticky. Can you publish a branch with your changes?

@julianxhokaxhiu
Copy link

I tested it locally, and it didn't work. So no need to push a commit that is useless.

Feel free though to try on your side. Remember that you can try locally by using npm run serve

@yyx990803
Copy link
Contributor

yyx990803 commented Jan 20, 2018

Just to explain why #1273 was needed instead of doing this: the webpack/babel transpilation only affects the bundles used in live mode. When the user uses WDS via the CLI with --inline flag, the client is injected by adding an entry to the users' bundle. So this PR would be an incomplete fix.

@SpaceK33z
Copy link
Member

Closing because #1273 fixed the same issue.

@SpaceK33z SpaceK33z closed this Feb 25, 2018
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

5 participants