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

Support webpack@4 and a few other things. #61

Closed
wants to merge 10 commits into from

Conversation

izaakschroeder
Copy link
Contributor

@izaakschroeder izaakschroeder commented Feb 7, 2018

I need to test this a bit more first, but you're welcome to have a look at it. Details are in the commits. This works with my setup but it should have the tires kicked with others before going out into the wild.

Resolves:

Newer versions of `webpack` use this to determine certain behaviours when embedding a module into the bundle. While there is no native support for CSS currently this won't break anything.
Most of these were never used, and the one that was (`isString`) isn't really worth the effort to maintain. This is especially true if one wishes to use `flow` later on to do type refinement.
Several of webpack's internal functions have changed. This changed functionality has been incorporated into the plugin.
This means you don't need to set the `publicPath` option during setup and the loader will use webpack's native one. Resolves faceyspacey#51.
This `options` parameter was set in `webpack-1` and even then it was set to be deprecated. I don't think this is used for anything anymore.
Still falls back to `process.env.NODE_ENV` check if no `hot` option is given.
This check fails when doing `npm link` or when there's any `webpack` instance mismatch. This isn't necessarily the same as a version mismatch, just that the `Chunk` class came from two different source files. This obviously happens during `npm link` when both folders contain copies of the `webpack` module. This looser check should still be ok and catch most cases where something bad is going to happen.
@izaakschroeder
Copy link
Contributor Author

/cc @GuillaumeCisco if you want to try this with your setup that would be 👌

@izaakschroeder
Copy link
Contributor Author

Will also have to follow webpack/webpack#6448 which may make this plugin not necessary for webpack@4.

@ZauberNerd ZauberNerd mentioned this pull request Feb 12, 2018
7 tasks
extractedChunk.addChunk(extractedChunks[chunks.indexOf(c)]);
});
chunk.parents.forEach(function(c) {
extractedChunk.addParent(extractedChunks[chunks.indexOf(c)]);

Choose a reason for hiding this comment

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

You'll need to pass down extractedChunks to this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PAkerstrand Good catch. It would be nice if there was a way to smoke test this against older versions. Thanks!

@GuillaumeCisco
Copy link

GuillaumeCisco commented Mar 2, 2018

Hey @izaakschroeder
Thank you for your time for migrating towards webpack 4.
I've just tested the code with my project and I had an issue which I thought was about webpack.
So I've created an issue on the webpack repo here : webpack/webpack#6655

Unfortunately, the errors was with the current code of this PR.
I let you take care of the details.

Cheers,

@GuillaumeCisco GuillaumeCisco mentioned this pull request Mar 5, 2018
@dawidvdh
Copy link

Is there any updates on this? Would really love to see webpack 4 support...

@faceyspacey
Copy link
Owner

@izaakschroeder thanks for all your work on this PR. My time has been limited lately, and with improved code-split CSS having made great progress by the webpack team, I haven't been able to prioritize this.

I'm not exactly sure what should be done, but it seems like more usage by others, as you pointed out, still needs to happen. That said this was a fork of the original plugin, so if anyone finds this useful, feel free to fork and so on.

@izaakschroeder izaakschroeder deleted the webpack-4 branch April 23, 2018 21:20
@ScriptedAlchemy
Copy link
Collaborator

I’ve resolved this issue. Please refer to for details #60 (comment)

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.

6 participants