Dedupe regression with browserify v8

[jgoz]

I'm logging the issue here, but the error is due to changes in browserify 8.0.0.

This might be best explained with an example:

• Module A depends on module B
• Other modules depend on module B', which is identical to B but a separate copy exists for whatever reason
• B is deduped by browserify and points to B'
• B gets routed to the A bundle, assigned ID of 200
• B' gets routed to the common bundle, assigned ID of 100
• Page includes common bundle then A bundle

A bundle in browserify v7:

200:[function(require,module,exports){
module.exports=require(100)
},{"dup":100}]}

A bundle in browserify v8:

200:[function(require,module,exports){
arguments[4][100][0].apply(exports,arguments)
},{"dup":100}]}

This results in an exception Uncaught TypeError: Cannot read property '0' of undefined because ID 100 is not defined in the current bundle.

This worked in v7 because it used require, which tries to resolve the module with previously defined requires from other bundles.

I realize this is an edge case and that if npm is correctly deduping dependencies, this situation should not occur. And the v8 behaviour is more correct in theory because B may have different dependencies from B'. However, assuming that any module is defined in the current bundle is dangerous when factor-bundle is involved.

[terinjokes]

Here we end up deduping with npm, rather than relying on Browserify (since in previous versions, we ended up with suboptimal bundles). I can put in the README this as a suggestion.

I can dig into the factoring to see if there's anywhere that we can detect duplicates and handle properly. Mind creating a failing test case for this?

[jgoz]

Here we end up deduping with npm, rather than relying on Browserify (since in previous versions, we ended up with suboptimal bundles). I can put in the README this as a suggestion.

I agree, that's by far the best way to do it. Putting it in the README is a good idea.

I can dig into the factoring to see if there's anywhere that we can detect duplicates and handle properly. Mind creating a failing test case for this?

From what I could tell, it was hard to reliably detect this scenario due to row ordering, but I will come up with a minimal test case later today.

[terinjokes]

Mentioning @substack to see if he hasn't any ideas.

[malte-wessel]

I'm also experiencing this issue. Is there a workaround to fix it?

[zoubin]

+1

This worked in v7 because it used require, which tries to resolve the module with previously defined requires from other bundles.

By the way, what's the problem with require?

I haven't found more details except https://github.com/substack/node-browserify/commit/324991581baf2797794207efc42ab18957635182

[Avnerus]

+1 I 'm also experiencing this issue and running npm dedupe doesn't seem to help. Only reverting the aforementioned commit fixes the problem.

[jesstelford]

I'm running into this issue now. Anyone got any ideas on how to fix it other than reverting the commit in browserify?

[Avnerus]

you could also override the browserify.prototype._dedupe function in your user code, but that's also not a very legitimate solution..

[d0b1010r]

This is regularly breaking the builds in our setup.

[terinjokes]

@substack can you take a look at this?

[darrennolan]

Also running into this today.

@terinjokes - Can you please elaborate on

Here we end up deduping with npm, rather than relying on Browserify (since in previous versions, we ended up with suboptimal bundles). I can put in the README this as a suggestion.

How do you go about doing this? I didn't see anything in the docs just yet about deduping flags, either to browserify or factor-bundle.

I'm just using cli for our builds, only deduping methods I saw were of the API.

Edit: Just further to this, I was previously mucking about with force-dedupe-git-modules in the past. This has resolved my issue. (Problem I think stems from one of my top level bundles is also partially included in another top level bundle. Using git for these dependencies doesn't always work great as they're always checked out again, even if the same tag).

But for the moment, this appears to be working for us.

[tellnes]

I've written a plugin which will probably solve this. It removes duplicates files completly from the pipeline right before browserify's dedupe.

https://github.com/tellnes/prundupify https://www.npmjs.com/package/prundupify

[magalhini]

@tellnes I can't thank you enough, in this world, for your brilliant, lovely, life-saving prundupify plugin. I've been banging my head on this issue for almost 4 days now, and your solution was the only one that magically made everything work again. Since what it does is pretty low-level for me, I'm struggling to understand exactly what's going on.

If you could shine some light as to what it's exactly doing and how it's magically fixing this issue, I'd be very very grateful. Not that I am not already, trust me, very much so :smile:

[tellnes]

Hi @magalhini and thanks for the feedback.

Browserify already sorts and marks all files that are duplicates. Then it removes the source code of the duplicate files, but executes it in different context for each different original file. That means that require('./a') !== require('./b/') is not true even if the source code is the same. To compere will require('./a') === require('./a') be true because the require statement will only execute the source code once and cache the return value from module.exports.

What prundupify does is to rewrite the resolved filepath for all files depending on b.js to what was resolved for a.js and completly remove b.js from the build. That means the same cached version will always be returned as long as the source code is the same. Yes, this might break some code, but it is probably what you expect from a deduper and what you want in a lot of cases for client side code.

Did that make sense? Btw, I'm open for pull requests or other proposals to improve the documentation for the plugin. English is not my native language and documentation is not my strongest skill.

[magalhini]

@tellnes Understood clearly, at least the theory of it :) thank you so much for taking the time to explain it in detail. I'll later follow along prundepify's code and try to make sense of it; hopefully even being able to suggest improvements.

So far I haven't encountered any issues with the plugin, so a double thumbs up. I'll keep you posted if I run into any issues!

[afanasy]

I'm having the same issue

[mourner]

Just ran into this bug as well while working on https://github.com/mapbox/workerpoolify. This should really be filed in the browserify repo.

[afanasy]

I'm currently using the following work-around (hooking into threshold):

plugin('factor-bundle', {
  threshold: function (row, groups) {
    if (row.expose == 'missingModule')
      return true
    return this._defaultThreshold(row, groups)
  }
})

[futpib]

I'm using the following workaround

b.pipeline.get('dedupe').push(through.obj(function (row, enc, next) {
    if (row.nomap) {
        row.source = 'module.exports = require('
            + JSON.stringify(row.dedupeIndex || row.dedupe)
            + ')'
        ;
    }
    this.push(row);
    next();
}));

Which is basically reverting substack/node-browserify@3249915 .

EDIT:

This resulted in other issues in my project, so I instead opted for dedupe: false option.