4.0: Multiple Names for one function

[SyntaxRules]

And example of this behavior:

//gulpfile.js
function nameTest(done) {
    done();
}

gulp.task('name-test', nameTest);
gulp.task('name-test2', nameTest);
gulp.task('name-test3', nameTest);
gulp.task('name-test4', nameTest);

//output
$ gulp --tasks-simple
name-test4
name-test4
name-test4
name-test4

$ gulp -T
[15:11:08] Tasks for gulpfile.js
[15:11:08] ├─┬ name-test4
[15:11:08] │ └─┬ name-test3
[15:11:08] │   └─┬ name-test2
[15:11:08] │     └── name-test
[15:11:08] ├─┬ name-test4
[15:11:08] │ └─┬ name-test3
[15:11:08] │   └─┬ name-test2
[15:11:08] │     └── name-test
[15:11:08] ├─┬ name-test4
[15:11:08] │ └─┬ name-test3
[15:11:08] │   └─┬ name-test2
[15:11:08] │     └── name-test
[15:11:08] └─┬ name-test4
[15:11:08]   └─┬ name-test3
[15:11:08]     └─┬ name-test2
[15:11:08]       └── name-test

I'd like to be able to alias tasks with multiple names and it doesn't seem possible right now. A use case for this is when a user was to make task A the default task as well like so:

gulp.task(A);
gulp.tasks('default', A);

A's name turns to default :(

[tunnckoCore]

A use case for this is when a user was to make task A the default task as well like so:

cant got it, but the first example definitely not looks okey to me.
i think output should not be like it is now. my expectation would be

//output
$ gulp --tasks-simple
name-test
name-test2
name-test3
name-test4

[yocontra]

Yeah this isn't the right behavior, it seems like gulp.task is mutating the function

[phated]

This is due to storing tasks in a weakmap, no idea how to change it. PRs welcome.

[stanier]

+1 for solution.

I'm not too familiar with maps, so I'll just voice my support for a fix and leave it alone.

[szwacz]

I started looking into it, and it seems that task aliases have been defined fine. The problem is ony with displaying them via undertaker.tree(). And yes, this is because of WeakMap use.

@phated I have few questions about design decisions to fix that

1. The easiest solution is to change WekMap to Map (Map has forEach, so this is piece of cake then). But....
2. In Undertaker the WeakMap is declared as singleton (https://github.com/phated/undertaker/blob/master/lib/helpers/metadata.js). Is this a design decision? For this code to work with Map metadata has to be changed to be constructed with every instance of Undertaker.

[phated]

Map shouldn't be used because we don't want to hold function references if they are cleaned up throughout a program. I didn't explicitly decide to make it a singleton but it shouldn't be exposed to consumers.

[daizenberg]

There's a couple of ways to resolve the issue. The key questions is - how should Undertaker behave when being fed with tasks referring to the same JS function. I can think of several approaches:

1. Prohibit such duplicates (why would we prohibit?)
2. Store them as separate tasks (requires to use {name, fn} as a key for WeakMap, which kills all advantage of the WeakMap)
3. Determine duplicate cases, and fold them into one task, which has array of aliases instead of name.

Each way has pros and cons. I vote for (3). I committed PR for Undertaker (gulpjs/undertaker#34)

[nmccready]

I have been using gulp.series or gulp.parallel for quite a while now for aliases. Why not adopt this behavior?

var gulp = require('gulp');


function nameTest(done) {
    console.log("task run");
    done();
}

gulp.task('name-test', gulp.series(nameTest));
gulp.task('name-test2', gulp.series(nameTest));
gulp.task('name-test3', gulp.series(nameTest));
gulp.task('name-test4', gulp.series(nameTest));


gulp.task('default', gulp.series('name-test'));

output

╰─⠠⠵ gulp -T
[21:50:41] Tasks for ~/code/github/nmccready/gulpplay/gulpfile.js
[21:50:41] ├─┬ name-test
[21:50:41] │ └─┬ <series>
[21:50:41] │   └── nameTest
[21:50:41] ├─┬ name-test2
[21:50:41] │ └─┬ <series>
[21:50:41] │   └── nameTest
[21:50:41] ├─┬ name-test3
[21:50:41] │ └─┬ <series>
[21:50:41] │   └── nameTest
[21:50:41] ├─┬ name-test4
[21:50:41] │ └─┬ <series>
[21:50:41] │   └── nameTest
[21:50:41] └─┬ default
[21:50:41]   └─┬ <series>
[21:50:41]     └─┬ name-test
[21:50:41]       └─┬ <series>

╰─⠠⠵ gulp --tasks-simple
name-test
name-test2
name-test3
name-test4
default

[erikkemperman]

I was looking at Undertaker metadata, and ran into this issue as well.

Clearly a lot of thought and careful consideration has gone into all this, and I guess I'm just missing something fairly obvious somewhere... So let me just throw this out here, hoping someone can make me see the light:

• Is the problem with aliases actually due to the WeakMap? Seems to me a regular Map would have the same issues? To my mind, the root cause here is that the task registry is keyed on name (meaning that for the aliases discussed here it will have several entries mapping to a single function) whereas the metadata is keyed on function (meaning that for aliased functions the metadata will only keep the most recently added name).
• Having said that, does the WeakMap actually achieve what it's supposed to here? I mean, I understand the principle of the thing, but won't there always be (strong) references to the function -- if only just those in the task registry? I suppose I simply lack the imagination to come up with a scenario in which entries in the metadata are actually garbage collected automatically. Unless... Do we expect custom registries to implement remove or clear?

[phated]

@erikkemperman thanks for digging into this deeper.

You are correct that Map and WeakMap would have the same problem because the key would be the function reference. The reason metadata is keyed by the function is that task names aren't unique; e.g. passing a function to gulp.series uses the function displayName/name as the task name, even if you never registered it.

The WeakMap is used because Undertaker makes no assumptions about how a registry stores tasks (though the default is in an object). I don't think that registries should have remove/clear but that isn't to say they can't, as Undertaker only looks for the methods it cares about and allows the rest. Undertaker was mainly written for usage inside gulp, but it should be generic enough to use in other scenarios (who knows, maybe even grunt can be powered by it).

[erikkemperman]

@phated Thanks for detailed response, that pretty much clears it all up for me.

Have an idea about this, going to be playing around in my fork with that. Sorry to be imposing on your time a bit much lately -- I'm having a little gap in between projects at work, and wanted to test the waters around Gulp#4.0 in preparation for the next one coming up.

I trust you'll feel free to ignore me if it's at all inconvenient :-)

[yocontra]

@erikkemperman Awesome, love having new contributors come in to poke around! Feel free to ping any of us on IRC if you have questions

[nmccready]

@erikkemperman whats your ideas? Love to chat and help you on it. I wouldn't mind jumping into that code base as well.

[erikkemperman]

@contra Good to know, thanks!

@nmccready Well the idea is decidedly half-baked at present, and might very well turn out to not work at all. Didn't have any time to try it out today unfortunately.

But very roughly, my thinking was that the metadata might be split in two maps.

The first map would map function keys to an object listing aliases, e.g.

{
  foo: <fn-foo-alias>,
  bar: <fn-bar-alias>
}

where each xx-yy-alias is a unique object.

The second map would reflect the topology, or tree structure, simply by having each parent-alias map to an array of child-aliases.

Anyway, the point is that we don't just record which functions are in the tree at a given point but also which name/alias was used to get it there (if any). Then we should be able to get tree() to fill in the appropriate labels in the appropriate places. Or so I hope.

[erikkemperman]

@nmccready Scratch that.

It's not going to work, is it? Because even if it fixes the output of gulp -T, that is only half the problem..

The other half being the output of running tasks, i.e. regular gulp. That relies on the extensions argument to bach, which does a callback passing only the function. So at that point we'll have no (obvious) way of figuring out where we are within the tree.

So... Apologies for inflicting this on you guys. Will think about it some more.

[phated]

@erikkemperman thanks for all the work you are putting into this. It is one of the biggest hangups currently and I don't have the time to dig into it. Keep bouncing ideas off and I can respond when I get a chance.

[erikkemperman]

OK, so I think I finally figured this out, more or less.

Submitted a PR with the magic number #42.

Added some tests and rebased to remove some unrelated commits that I'd let slip in there earlier. New PR is #43.

[phated]

This was fixed by gulpjs/undertaker#50 and will be released in the next version of Undertaker.

[nmccready]

👍

[phated]

This just landed in the 4.0 branch. There was a breaking change in undertaker 0.13 so I bumped the gulp version to 4.0.0-alpha.2 and added support for the new version in gulp-cli also. Please be sure to update everything to get this fix.

[tunnckoCore]

Awesome! 👍