browserify / factor-bundle

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

Fix for factor-bundle + watchify + gulp 'EventEmitter memory leak detected' issue #65

Closed ypt closed 8 years ago

ypt commented 9 years ago

Implemented the fix for #64 suggested by @Freyert

rtibbles commented 9 years ago

Could this be merged? Pretty handy to have it in master.

d0b1010r commented 9 years ago

+1

Freyert commented 9 years ago

proud

zoubin commented 9 years ago

Right now you can specify opts.outputs as a function, and thus no need to plugin factor-bundle each time reset.

I have tried some way to use watchify with factor-bundle, no memory leak detected.

I think plugging factor-bundle repeatedly seems a little odd, because reset only rebuild the pipeline, and all other plugins are plugged only once.

davidchase commented 8 years ago

@zoubin you are correct i tired it out by moving factor-bundle outside of my re-bundling method and it works therefore this fix is un-needed at least for me as well.

my simple setup using browserify-incremental + chokidar

var bundleStream = browserifyInc(files, {
    cache: {},
    packageCache: {},
    fullPaths: true,
    cacheFile: 'browserify-cache.json'
}).plugin('factor-bundle', {
    outputs: outputFiles
});

var bundleScripts = function bundleScripts() {
    bundleStream
        .bundle()
        .pipe(fs.createWriteStream('./client/dist/js/common.js'));
};
rtibbles commented 8 years ago

I think this may be my issue as well.

ypt commented 8 years ago

Alright, it sounds like this fix isn't really necessary, and @zoubin and @davidchase have solutions. I'll close this PR.