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 a significant patch about manifest separating #17

Merged

Conversation

e-cloud
Copy link
Contributor

@e-cloud e-cloud commented Jun 30, 2016

this is a big PR!

e-cloud added 9 commits June 29, 2016 18:58
because inside `findMatchingBucket`, all the `userRequest` manipulation are just for retrieve the path as `resource`
1. move the bucketChunks management out of the entry chunk loop which makes more sense.
2. create a manifest chunk to act as the only entry chunk.

see discussion in BohdanTkachenko#7

ps: improve example's html script entries
@e-cloud
Copy link
Contributor Author

e-cloud commented Jun 30, 2016

@BohdanTkachenko , could you review this pr 😃

@BohdanTkachenko
Copy link
Owner

Wow! Looks great! Will review it.

@BohdanTkachenko
Copy link
Owner

It seems that this PR breaks html-webpack-plugin.
In my project it says Error: The loader "/Users/bohdantkachenko/Projects/test/node_modules/html-webpack-plugin/lib/loader.js!/Users/bohdantkachenko/Projects/test/src/index.ejs" didn't return html.

@e-cloud
Copy link
Contributor Author

e-cloud commented Jul 3, 2016

@BohdanTkachenko , could you provide some example code to test?

And, analyzing from your error above, it seems that ejs file is not converted to hml string before being passed to the html-webpack-plugin. Coud you add something like ejs-loader or so?

@BohdanTkachenko
Copy link
Owner

You can find an example here: https://github.com/BohdanTkachenko/webpack-split-by-path-error-demo-1

Also, I changed from ejs to html and it is the same. Also if you switch to master branch of webpack-split-by-path - everything works.

e-cloud added 2 commits July 4, 2016 01:38
if not doing so, other plugins like `html-webpack-plugin` will crash as
we extract its inner chunks's modules' dependencies outside.
As Html-webpack-plugin runs a child compiler, which initiated
with part of parent compiler's plugins by webpack. If we listen
on `compilation`, handlers in `webpack-split-by-path` will be
copy to the child compilation instance, which cases some unwanted
extraction.

So, we just listen to `this-compilation`.

See
https://github.com/webpack/webpack/blob/webpack-1/lib/Compiler.js#L327-L328
and
https://github.com/webpack/webpack/blob/webpack-1/lib/Compiler.js#L363-L364
@e-cloud
Copy link
Contributor Author

e-cloud commented Jul 4, 2016

@BohdanTkachenko i just solved the problem with some changes. It may break compatibility.

The reason that the master doesn't have problem, is that it doesn't add a chunk outside chunk filtering logic which doesn't impact the child compilation with just skipping.

@e-cloud
Copy link
Contributor Author

e-cloud commented Jul 6, 2016

@BohdanTkachenko, do you have time to review?

@BohdanTkachenko BohdanTkachenko merged commit 675c9a4 into BohdanTkachenko:master Jul 6, 2016
@BohdanTkachenko
Copy link
Owner

Tested - seems to be working. Merged and published to NPM as version 0.1.0
Thanks for your awesome contribution!

@e-cloud
Copy link
Contributor Author

e-cloud commented Jul 7, 2016

it is normal that user might use multiple instance of Webpack-split-by-path? Current implementation doesn't support multiple instance as manifest is the only entry chunk that the non-first instances actually do nothing.

@BohdanTkachenko
Copy link
Owner

I don't think that it is normal because webpack-split-by-path supports several bundles in configuration, so no need in several instances of a plugin.

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.

2 participants