-
Notifications
You must be signed in to change notification settings - Fork 37
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
Avoid unnecessary require
s
#7
Conversation
@@ -24,24 +24,29 @@ var webpackBootstrapFunc = function(modules) { | |||
} | |||
|
|||
module.exports = function (fn) { | |||
var fnString = fn.toString(); |
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.
Moving this line because it avoids stringifying the function numerous times.
Thanks ! available in version 1.1.0. Please take a look at the caveat I added to the README. |
Ah! Good thinking about the caveat. I forgot about that option in webpack. |
Is there any way to get around this? Builds are now 60+ seconds when they used to be 400ms because of this :( |
@contra please give more info. |
@borisirota You can't use efficient sourcemaps after this was merged, there's a note in the README about it. It does affect webpack build perf by placing that limitation on anyone who uses it. |
@contra you are right and you can get around this by using v1.0.6. |
@contra in the new v2 there is no limitation on the |
We encountered a problem with
webworkify-webpack
. Byrequire
ing every module, it is forcing all modules to be initialized. Some of our modules have assumptions about when they are initialized. In particular, they have to be initialized after we use something that useswebworkify-webpack
as a dependency.To fix this, I purpose that we avoid
require
ing the modules by doing a simple check to see if the module "looks like" the right one. MakingwrapperFuncString.indexOf(fnString) > -1
a necessary condition for the require address this issue. The subsequent logic is updated to behave appropriately since the conditions are now reversed.This fixes the problem and does not change existing behavior as far as I know. This can be considered a breaking change. However, it is one that reverts the behavior to what I and most users expect.