search

browserify / factor-bundle

factor browser-pack bundles into common shared bundles
Other
400 stars 27 forks source link

Use hash ids for pack output #21

Closed jgoz closed 10 years ago

jgoz commented 10 years ago

This fixes #13.

Instead of ints, this uses the browserify._hash method on module ids in both entry and common bundles. Common dependencies are added to browserify.exports to ensure their hashes are exposed (simulates using -x and -r to generate factored output).

The cost of improved accuracy is a few extra bytes per dependency.

Note that this requires browserify ^3.30.5, due to the use of ._hash. I took the liberty to update some dependencies in this PR, but I can revert that if you like.

sixtimes commented 10 years ago

i'm still trying to diagnose this, but after this executes correctly the first time with browserify, it consistently crashes a second time using substack/watchify, with the following relatively unhelpful console message:

events.js:72 throw er; // Unhandled 'error' event ^ Error: ENOENT, open 'eeO1y8'

i'm guessing that means it's trying to open a key somewhere instead of a file path, but i haven't yet been able to identify where.

jgoz commented 10 years ago

Hmm, I didn't test it with watchify - thanks for doing that. I'll look into it too.

ghost commented 10 years ago

Merged in 1.0.0. Thanks for the patch! watchify hasn't been made to work with factor-bundle yet so this patch doesn't affect that.

jgoz commented 10 years ago

@substack

watchify hasn't been made to work with factor-bundle yet so this patch doesn't affect that

On that subject, I spent a bit of time debugging tonight and figured out how to make it work with watchify. Whether it fully benefits from the caching that watchify provides is another question.

Anyway, there were 2 issues with the current implementation:

  1. The through stream on bundle that updates rmap was changing the ids of the original rows that were getting passed through, which was messing with watchify's cache. Cloning the rows in the through stream fixes that.
  2. The stream returned by factor-bundle ends after the first call to bundle, so further calls will not trigger the stream and data events, producing an empty common bundle. This is solved by re-initializing the stream when it ends. It's kind of a hack and I'm not super happy with it, but it works.

My solution is pretty messy at the moment, so it might take a larger scale cleanup before submitting a patch.