- Notifications
You must be signed in to change notification settings - Fork59
Prevent chunking out when not in production environment#50
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jsf-clabot commentedApr 26, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
michael-ciniawsky left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@jalooc I'm not regularly using this loader so I need to get other eyes on this aswell 😛
| } | ||
| varresult; | ||
| if(query.lazy){ | ||
| if(process.env.NODE_ENV!=='production'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think it's better to introduce an option here,NODE_ENV === 'production' is common, but no real standard
jalooc commentedApr 28, 2017
I was also wandering about adding an option(just another env var) forcing synchronous load, which could be potentially useful in static page rendering e.g. for ssr or bots. Do you think it's a good idea? |
michael-ciniawsky commentedApr 28, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@jalooc If you are not enterily pissed of in case we maybe need to revert a few things afterwards, then please go ahead and we will see if it is something we should add or not 😛
Just and option (query), eg entry.js importbundlefrom'./name.bundle.js' webpack.config.js {test:/\.bundle\.js$/,use:{loader:'bundle-loader',options:{name:'[name]'}}},{test:/\bundle\.async\.js$/,use:{loader:'bundle-loader',options:{lazy:true,name:'[name]'}}} etc 😛 Maybe if we add a |
joshwiens commentedMay 2, 2017
michael-ciniawsky commentedMay 3, 2017
kk, so sync doesn't make any sense at all ? I was wondering about it, but not 💯 since i'm not using this loader extensively. If so we better close the PR :) |
jalooc commentedMay 7, 2017
@michael-ciniawsky I'm not sure if I fully understand your point. The suggested in this PR ability for skipping chunking-out (I called it sync loading before, but maybe it was confusing) is just for development purposes, so files naming convention wouldn't be right for this application. I also don't think it's related with#51. |
Due to HMR problems with on-demand chunks, I figured it would be nice if the loader loaded the chunks synchronously when in dev environment and as so far in prod. And that's what this commit does.
Other ideas: I was also wondering about adding some additional flag toforce async loading in development, but eventually opted-out. Alternatively, sync loading in development could be made an optional (instead of default) behavior. If you find some of the modifications useful, I'd be happy to alter the merge request to suit more needs.