Crashes when used with watchify.

[englercj]

This is a really great plugin, but I'm getting an issue where when using watchify and incremental build crashes.

Here are my versions:

├── browserify@11.0.1
├── pathmodify@0.4.0
└── watchify@3.3.1

Here is the error:

...\node_modules\pathmodify\index.js:88
    var
    ^

RangeError: Maximum call stack size exceeded
    at alias_resolver (...\node_modules\pathmodify\index.js:88:5)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)
    at alias_resolver (...\node_modules\pathmodify\index.js:126:12)

Here is my config:

{
    mods: [
        pathmodify.mod.id('_env/config', path.join(BASE, './src/app/js/config/' + env + '.js')),
        pathmodify.mod.dir('_app', path.join(BASE, './src/app/js')),
        pathmodify.mod.dir('_plugins', path.join(BASE, './src/plugins'))
    ]
}

Hopefully this makes sense, looks like there is some recursive loop that happens when watchify tries to do an incremental build.

[jmm]

@englercj Thanks for the compliment and report.

Can you show me more of the code around that config snippet? Do you by chance have something like this:?

var b = watchify(browserify(/* ... */)).on("update", function () {
  b
    .plugin(pathmodify(), /* ... */)
    .bundle()
    /* ... */
});

[englercj]

Yes, except I do not register the plugin inside the update function. More or less for my watch taks I do this:

watchify.args.debug = true;

var bundler = watchify(browserify(watchify.args));

bundler
    .on('update', function () {
        doBundle(this);
    })
    .external('something')
    .plugin(pathmodify(), config.browserify.pathmodify)
    .transform(hbsfy)
    .add(path.normalize(filePath));

doBundle(bundler);

function doBundle(bundler) {
    bundler
        .bundle()
        .pipe(source(outputName))
        .pipe(gulp.dest(config.paths.staticOut + '/js'));
}

So it does call .bundle() again on update, but not .plugin().

[jmm]

@englercj Thanks for the additional info. This is puzzling. The stack trace makes is seem like the resolver function is being called with itself as the callback, but I don't know how that would happen. I'm not sure how to reproduce this. Is there some kind of consistency to it, like you start watching and do a certain number of updates (rebundles) and that triggers it? Are you able to create a minimal repo that reproduces it for you?

[englercj]

It happens 100% of the time on the first rebundle. The first build works normally, then the on update rebundle causes the crash. If I change any file when watching it will crash.

I can try to get a minimal repro case up, my current build process is much to complex to be useful for you.

[englercj]

Investigating now, but it seems the reason it recurses is because resolver inside of alias_resolver is being set to alias_resolver. Which means on line 126, when it calls resolver() it calls itself.

Investigating why this is the case now.

[englercj]

Looks like when the reset event is sent from browserify, this plugin calls .init(). When that happens deps.resolver is set to alias_resolver from before. The code there assigns that into opts which causes the recursion.

Should something different be happening on reset event?

[englercj]

Looks like the problem is watchify. Seems like there was a change to how watchify works somewhere between v3.3.1 and v3.6.0 that this plugin relies on.

Upgrading watchify to v3.6.0 fixed the crash.

[englercj]

Actually after upgrading to watchify@3.6.0 and browserify@12.0.1, the resolving no longer works on an incremental build. I get an error from browserify saying "can't find _plugins/something" which is one of the directory mods in my config.

[jmm]

@englercj Hmm, this is all very puzzling. I think I was using this with watchify@3.3.1, then 3.4.0. If you can put together a minimal repro (preferably a repo that I can just clone and run), that would be invaluable.

[englercj]

I've been trying to create a minimal case but unable to repro there so far. I actually think I just found the problem though. Let me test and I'll post.

[englercj]

The plugin doesn't copy the config object passed in, it actually modifies and uses it internally. This is OK when you only have a single bundle because the config is only ever passed to a single instance of pathmodify.

However I have multiple bundles, all of which share the pathmodify config. Since I pass it into each of my bundles they all share the config and cause all sorts of havoc. If I inline my config so that for each pathmodify() call I create a separate object, it actually works fine (browserify@11.0.1, watchify@3.3.1).

Basically this was happening:

var PATH_MODIFY_CONFIG = {....};

setupBundle('...');
setupBundle('......');

function setupBundle(filePath) {
    var bundler = watchify(browserify(watchify.args));

    bundler
        .plugin(pathmodify(), PATH_MODIFY_CONFIG) // now all the bundles have the same object for opts!
        .add(path.normalize(filePath));
}

[jmm]

@englercj Thanks for debugging this! I actually recently made a change in my local repo that creates a new internal options object (to avoid leaking data via the object passed in) which should fix this. I will try to repro with this information and confirm. I'm sure you don't need to be told this -- in case anyone else runs into it, for the time being using Object.assign() or whatever object extension function you prefer should work around it.

[jmm]

Even though I know what the problem is in pathmodify (using the passed in opts for internal state) I wanted to understand the whole problem more fully and I was having a hell of a time repro'ing this. I was trying to repro it legitimately with watchify and it was very sporadic. To repro it consistently I took watchify out of the picture: https://github.com/jmm/pathmodify-5-repro ($ npm i && node bundler). I'm not sure exactly how it works, but apparently with watchify the update events don't always fire on the bundles in the same order they were added. The pathmodify bug (this overt one anyway) rears its head when update fires on an earlier bundle after a later one. That's not a great description, but the repo shows what I mean.

[panzerdp]

Faced the same problem. The solution suggested by @englercj solved the problem, when copying the configuration object for pathmodify.

[jmm]

@englercj @panzerdp Sorry for the trouble.

This has been fixed in v0.5.0. Internal state is no longer leaked (and shared with other invocations) via the passed in options object. It wasn't involved in this bug, but I also made it so opts.mods can't be mutated after passing options to the plugin.

@englercj Thank you for reporting and debugging this! As I mentioned before, this was already fixed by a change in my local repo, but that was because I wanted to correct internal state leaking via the options object in general -- I wasn't aware of the severity of the problem. I've added regression tests for this specific bug:

https://github.com/jmm/pathmodify/blob/49da32a5dffeac1f8e97d96ce62ca814f2144c47/test/unit/test.js#L231 https://github.com/jmm/pathmodify/blob/49da32a5dffeac1f8e97d96ce62ca814f2144c47/test/integration/test.js#L625

Please note that in v0.5.0 the signature of the export has been changed from a factory function that returns the plugin function (require("pathmodify")()) to the plugin function itself (require("pathmodify")).

[englercj]

:+1: Nice dude, thanks for getting this fixed!

[panzerdp]

@jmm Yes, the version 0.5.0 works well now, thanks for fixing.