browserify / factor-bundle

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

Issue with broken dep #61

Open RichardLitt opened 9 years ago

RichardLitt commented 9 years ago

I am using browserify with brfs in a Chrome extension. My gulpfile looks like this:

gulp.task('browserify', function() {
  return browserify(paths.main)
    .transform(reactify)
    .transform({global: true }, 'brfs')
    .plugin('factor-bundle', {outputs: ['build/main.min.js', 'build/background.min.js']})
    .bundle()
    .pipe(source('common.min.js'))
    .pipe(gulp.dest('build/'))
})

In the bundle loaded into the Chrome extension, I have a viewer.html page, into which I have plopped:

<script defer src="../../common.min.js"></script>
<script defer src="../../main.min.js"></script>

In place of the previously working bundle.min.js. Now, however, on load, main.min.js no longer works, throwing a Uncaught TypeError: Cannot read property '0' of undefined error. The line it points to looks like this:

{"react/lib/ExecutionEnvironment":682,"react/lib/invariant":788}],280:[function(require,module,exports){
arguments[4][278][0].apply(exports,arguments)
},

I'm afraid I have no idea why this is happening, at all, to the point of not knowing how to even name this issue. I'm going to go back to not using factor-bundle for now. I'll have keep this issue in a branch, but I'm stumped.

terinjokes commented 9 years ago

@RichardLitt The stream you're returning from this gulp task finishes first, and gulp exits, The other streams are still being written, and what the fs sink has buffered is written out, but not the rest.

Do you really need to use gulp for this? It would be much simpler if you didn't.

RichardLitt commented 9 years ago

Thanks @terinjokes. What other streams are still being written? I thought this was all essentially one stream.

Gulp is used along with Watchify for a lot of other processes in my dev pipeline. I think it would be more awkward if I didn't use it. I may be wrong; here's the file I use.

justrhysism commented 9 years ago

@RichardLitt The other streams created by factor-bundle are outside of your Gulp stream. I was bitten by this too.

RichardLitt commented 9 years ago

Ahh. Can you help me by showing your solution?

justrhysism commented 9 years ago

With Gulp, you pretty much can't rely on the files being written. It's been a while since I investigated as due to a bug I've been unable to get factor-bundle to work reliably on Windows (which half of my team uses).

The only way I thought it may be possible (but was unable to get it working at the time, mind you, this was about 9 months ago) is if you can catch an event and pass it up the pipeline - and use that to "complete" the Gulp task. However, I was unsuccessful and I needed to move on: https://github.com/substack/factor-bundle/issues/20

terinjokes commented 9 years ago

I have an example from work I can clean up and provide tomorrow. Pretty much you need to combine the factor-bundle streams with the main browserify stream.

RichardLitt commented 9 years ago

Thanks @terinjokes! That'd be awesome.

jzeltman commented 9 years ago

+1 @terinjokes I'm encountering the same issue with gulp. Do you have that example?

robert-chiniquy commented 9 years ago

I've got the same issue also, using factor-bundle with gulp. Not convinced it is a gulp issue. This is with:

    "browserify": "^10.2.1",    
    "factor-bundle": "^2.4.0",
robert-chiniquy commented 9 years ago

Is this definitely a different issue from https://github.com/substack/factor-bundle/issues/51 ?

geirsagberg commented 9 years ago

I have a similar issue; @terinjokes could you show that example you mentioned?

jjgonecrypto commented 9 years ago

@terinjokes would appreciate seeing that example as well.

jjgonecrypto commented 9 years ago

For those interested, what you really need to know is when all of the output streams have finished.

So if factor-bundle instead of this: https://github.com/substack/factor-bundle/blob/master/index.js#L94 did

Object.keys(pipelines).forEach(function (id, ix) {
    b.emit('factor.pipeline', id, pipelines[id], outputs[ix]);
});

then you'd have a mechanism to listen outside of factor-bundle when those streams were complete (by listening to all of them finish, eg.

var ps = [];
b.on('factor.pipeline', function (file, pipeline, output) {
    ps.push(pipeline);
    output.on('finish', function () {
        ps.pop();
        if (!ps.length) cb(); // now all factor bundles are complete       
    });
});

However, this isn't very stream-y. As for what @terinjokes said:

Pretty much you need to combine the factor-bundle streams with the main browserify stream.

that's where my limited streams experience is throwing me, as I'm at a loss as to what to push into the browserify pipeline such that it only emits and end when the main b.pipeline and all of the output streams have finished.

Any suggestions?

islemaster commented 8 years ago

I'm running into a similar issue with knowing when factor-bundle has completed its work. You can do something similar to what @justinjmoses suggests by building your own streams (so you can listen on their events) and passing them as the outputs argument:

function makeWriteStream(filename) {
  return fs.createWriteStream(filename).
      on('finish', function () {
        console.log('Finished writing ' + filename);
      };
}

var b = browserify(inputFiles);
b.plugin(factor, { outputs: inputFiles.map(makeWriteStream) });
b.bundle().pipe(makeWriteStream('common.js'));

That, while not particularly stream-y, at least gives you a hook to know when all files are done writing.

Unfortunately, I've discovered that you never get the 'finish' (or 'unpipe') event on factor-bundle's streams if a browserify error occurs (for example, a syntax error in the JS you're trying to bundle). I've written a failing test here: https://github.com/substack/factor-bundle/compare/master...islemaster:streams-dont-close

jjgonecrypto commented 8 years ago

FWIW @islemaster, I'm using this slight fork of factor-bundle to get the output streams a an extra parameter of the factor.pipeline event.

https://github.com/justinjmoses/factor-bundle

IagoSRL commented 7 years ago

This sure comes late, but may help others or to create a built-in solution

I found myself with the commented problem about the ending of factorized streams, but I tried listening to the end event in the pipeline object (given by factor.pipeline) and it works (as of v2.5.0), so I don't need a fork to listen at the output object. My knowledge on streams is short, so not sure if has all the same 'effects' listen at pipeline or output, but looking a bit at the code of the [labeled-]stream-splicer seems to be the same, but I may be wrong.

I found this issue because of the common problem of don't know when an error happens, breaking all streams, but I have a solution now. It's possible to listen to an error event on the output object given by the bundle event in the browserify instance, just like:

browserify.on('bundle', function(output) {
    output.on('error', function(err) {
        // Stop task, warn about error
        grunt.verbose.error().error(err);
        grunt.fail.warn('Browserify bundle error: ' + err.toString());
    });
});

That will throw on errors that I think happens before factor-bundle creates streams. Additionally, I listen to error event on the pipeline objects.

I'm using Grunt, not Gulp, then the full set-up is completely different, but here is a link to the full code just for reference: grunt/browserify.js snapshot


Just a question/idea for maintainers: would be possible to add an API to factor-bundle to simplify all this? Maybe make it emits events at the 'browserify' instance for streams completion and bundle-error, or pause the main stream until the factorized streams ends.