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

fix for end event of bundle is never triggered #250

Closed
wants to merge 1 commit into from

Conversation

inkless
Copy link

@inkless inkless commented Jul 30, 2015

I come across an issue when the watchify upgrades to 3.3.1.
So I'm using grunt-browserify in my current build process, and I have a config like this:

var browserifyConfig = {
        watch: {
            files: browserifyFilesConfig,
            options: {
                // generate sourceMap, helps debug
                browserifyOptions: {
                    debug: true
                },
                watch: true,
                keepAlive: true
            }
        }
    };

Somehow, when I change any file, the watch is never triggered. After I dived into it, it seems

bundle.on('error', onend);
bundle.on('end', onend);
function onend () { updating = false }

onend function is never triggered.
But if I add another event: bundle.on('data', function() {});, it fixed the bug. and onend is triggered, not sure why this happens, but it does help fix the problem.

@ghost
Copy link

ghost commented Jul 30, 2015

If the bundle isn't consumed, it won't emit an end event. I wonder how common this is? It feels messy to put bundle into flowing mode (and might create other problems with ignored chunks) but we might need to do that here.

@inkless
Copy link
Author

inkless commented Jul 30, 2015

It happens 100% of time, it happens both in my colleague's laptop and mine. I don't have this issue before, I was using 3.2.3 at that time. But then after I also upgrade it to 3.3.1, it just doesn't work. Still not sure why this happens.
Btw, I'm using grunt-watchify 3.8.0, node v0.12.7, npm 2.11.3, grunt v0.4.5

@ghost
Copy link

ghost commented Jul 30, 2015

@inkless what version of browserify are you running? 11.0.1 has a small patch that might be related.

@inkless
Copy link
Author

inkless commented Jul 30, 2015

@substack It seems both 10.2.4 and 11.0.1 doesn't work for me... :(

@ghost
Copy link

ghost commented Jul 30, 2015

I'm still not sure why this is causing issues. When I do what grunt-browserify does here: https://github.com/jmreidy/grunt-browserify/blob/master/lib/runner.js#L129-L141

var watchify = require('./')
var browserify = require('browserify')

var w = watchify(browserify(process.argv[2]))
w.on('update', function () {
  w.bundle(onbundle)
})
w.bundle(onbundle)

function onbundle (err, src) {
  if (err) console.error(err)
  else console.log(src.length + ' bytes')
}

This works fine. Maybe grunt is doing something else strange? I'm not familiar with how to set up a grunt build, so if somebody could verify that a plain grunt-browserify build fails, that would be useful information.

@zertosh
Copy link
Member

zertosh commented Jul 30, 2015

@inkless Can you put together a small repo with the issue so I can try to debug it?

@inkless
Copy link
Author

inkless commented Jul 30, 2015

https://github.com/inkless/grunt-browserify-test
I created a repo, I think you can reproduce it in that repo.

@zertosh
Copy link
Member

zertosh commented Jul 30, 2015

@inkless Thanks for the repo, it helped me figure out the problem. I submitted a fix to grunt-browserify.

The problem is that because of how npm installs semver incompatible versions, grunt-browserify wasn't using browserify@11.0.1 as you expected, but rather v10. v11 has this fix browserify/browserify@90a429d for the problem you're having.

@zertosh
Copy link
Member

zertosh commented Jul 30, 2015

@inkless grunt-browserify@3.9.0 fixes this issue.

@zertosh zertosh closed this Jul 30, 2015
@inkless
Copy link
Author

inkless commented Jul 30, 2015

Cool thanks!

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

Successfully merging this pull request may close these issues.

2 participants