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

Handle commonschunkplugin assets #64

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

steven-haddix
Copy link

Currently, commonschunkplugin will pull vendor libraries into a separate asset and use a manifest file to pull assets on demand. Webpack does this by updating the main app asset to use a global webpackJsonp function to pull in dependencies. WebpackJsonp will not be available in the global context during eval and the build will fail.

I've added a function the does an eval on both manifest and vendor assets, then inserts those into the scope of the main app eval. This should make any vendor related assets available to the app during build time.

@coveralls
Copy link

Coverage Status

Coverage decreased (-17.6%) to 82.353% when pulling da2cfed on steven-haddix:commonschunk-loading into 2303b00 on markdalgleish:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-17.4%) to 82.558% when pulling 1bf2cf9 on steven-haddix:commonschunk-loading into 2303b00 on markdalgleish:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-18.4%) to 81.609% when pulling 963e197 on steven-haddix:commonschunk-loading into 2303b00 on markdalgleish:master.

@markdalgleish
Copy link
Owner

Coverage has dropped below the threshold. Any chance you could add tests for this?

@steven-haddix
Copy link
Author

Yeah I'll try to get some in within the next day or two.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c0d990e on steven-haddix:commonschunk-loading into 2303b00 on markdalgleish:master.

@markdalgleish
Copy link
Owner

markdalgleish commented Nov 24, 2016

I have a couple of concerns:

  • The chunks have been hard-coded as 'vendor' and 'manifest', meaning this can't work on arbitrary sets of chunks.
  • When I think this through further, you should probably be using a separate static render config without bundle splitting, rather than trying to get chunked output to run in a node context. This is something I should probably document, possibly even warn against if multiple JS chunks are detected.

@steven-haddix
Copy link
Author

steven-haddix commented Nov 24, 2016 via email

@peter-mouland
Copy link

peter-mouland commented Jan 26, 2017

is using context still the recommended way to handle the webpack.optimize.CommonsChunkPlugin ?

I haven't worked out how to get it working... new StaticSiteGeneratorPlugin('HelloWorld', data.routes, data, { webpackJsonp: (a)=> a })

i would still get this error: Export from "HelloWorld" must be a function that returns an HTML string

When i remove the commonChunksPlugin everything is cool.

any thoughts?

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

4 participants