-
Notifications
You must be signed in to change notification settings - Fork 59
Prevent chunking out when not in production environment #50
base: master
Are you sure you want to change the base?
Conversation
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 😛
@@ -19,7 +19,9 @@ module.exports.pitch = function(remainingRequest) { | |||
var chunkNameParam = ''; | |||
} | |||
var result; | |||
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
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? |
@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 import bundle from './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 |
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 :) |
@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 to force 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.