Skip to content
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

Issue with broken dep #61

Open
RichardLitt opened this issue Mar 7, 2015 · 16 comments
Open

Issue with broken dep #61

RichardLitt opened this issue Mar 7, 2015 · 16 comments

Comments

@RichardLitt
Copy link

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
Copy link
Contributor

@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
Copy link
Author

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
Copy link

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

@RichardLitt
Copy link
Author

Ahh. Can you help me by showing your solution?

@justrhysism
Copy link

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: #20

@terinjokes
Copy link
Contributor

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
Copy link
Author

Thanks @terinjokes! That'd be awesome.

@jzeltman
Copy link

jzeltman commented Apr 8, 2015

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

@robert-chiniquy
Copy link

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
Copy link

Is this definitely a different issue from #51 ?

@geirsagberg
Copy link

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

@jjgonecrypto
Copy link

@terinjokes would appreciate seeing that example as well.

@jjgonecrypto
Copy link

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
Copy link

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: master...islemaster:streams-dont-close

@jjgonecrypto
Copy link

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
Copy link

IagoSRL commented Jul 14, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants