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: webpack 4 #413

Merged
merged 2 commits into from Mar 8, 2018
Merged

fix: webpack 4 #413

merged 2 commits into from Mar 8, 2018

Conversation

@getkey
Copy link
Contributor

@getkey getkey commented Mar 6, 2018

This PR fixes API changes introduced in Webpack 4.

@tivac
Copy link
Owner

@tivac tivac commented Mar 6, 2018

@getkey Thanks! Is this actually testing against webpack@4?

Loading

@getkey
Copy link
Contributor Author

@getkey getkey commented Mar 6, 2018

Right, I forgot to run the tests. I also need to update the devDependency. I'll do it tomorrow because I'm in bed now. ;-)

Loading

@getkey
Copy link
Contributor Author

@getkey getkey commented Mar 7, 2018

A bunch of tests are failing because webpack@4 prepends ./dist/ to the output path.

Loading

@tivac
Copy link
Owner

@tivac tivac commented Mar 7, 2018

Neat, I'll take a look when I get into the office in a bit. May just need to tweak the test webpack configs, hopefully.

Loading

@tivac
Copy link
Owner

@tivac tivac commented Mar 7, 2018

Most of the failures seem to be around webpack@4 defaulting to production mode. I'm working on finalizing the test cleanup in a branch that I'll merge manually.

https://github.com/tivac/modular-css/tree/pr/413

Having some weird output from the test that runs webpack multiple times w/ changed file contents though, will need to investigate that further I think.

Loading

@tivac
Copy link
Owner

@tivac tivac commented Mar 7, 2018

Looks like compiler.plugin("invalid") is no longer triggering, so my plugin happily barfs out the cached contents forever.

😒

I'll need to do some research on what replaced that event. I love major version upgrades!

Loading

@deflock
Copy link

@deflock deflock commented Mar 7, 2018

Loading

@tivac
Copy link
Owner

@tivac tivac commented Mar 7, 2018

@deflock I tried that, it never seems to get invoked either. 😒

Loading

@tivac
Copy link
Owner

@tivac tivac commented Mar 8, 2018

Tracked it down, fixing the last few Travis failures now.

Loading

@tivac tivac merged commit 9c03206 into tivac:master Mar 8, 2018
1 check failed
Loading
tivac added a commit that referenced this issue Mar 8, 2018
#413 got things working, now the tests pass too!
tivac added a commit that referenced this issue Mar 8, 2018
tivac added a commit that referenced this issue Mar 8, 2018
#413 got things working, now the tests pass too!
@tivac
Copy link
Owner

@tivac tivac commented Mar 8, 2018

@getkey published as modular-css-webpack@8.2.0

Thanks again! 🎉

Loading

@getkey
Copy link
Contributor Author

@getkey getkey commented Mar 8, 2018

Awesome, thanks @tivac!

If I understand correctly #409, support for Webpack 2 & 3 is dropped for the moment, right?

Loading

@getkey getkey deleted the webpack-4-fix branch Mar 8, 2018
@tivac
Copy link
Owner

@tivac tivac commented Mar 8, 2018

AFAIK this version should be compatible with webpack 2-4, though testing it against multiple versions is challenge.

Loading

@getkey
Copy link
Contributor Author

@getkey getkey commented Mar 8, 2018

Ah alright, yes! 🙂

Loading

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

Successfully merging this pull request may close these issues.

None yet

3 participants