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

Decamelize tasks created by ES module exports #2270

Comments

@coreyfarrell
Copy link
Member

What were you expecting to happen?
ES modules cannot export names with dashes so I was expecting (hoping) camel case exports could be converted to dashed names (so npx gulp test-local with sample gulpfile). I know decamelizing would be a breaking change so I can understand if this cannot happen, just figured I'd post in case this would be acceptable for @next.

What actually happened?
All exports become tasks without modification of naming.

Please post a sample of your gulpfile (preferably reduced to just the bit that's not working)

export function testLocal() { return Promise.resolve(); }

What version of gulp are you using?
gulp@4

What versions of npm and node are you using?
npm: 6.4.1
node: 10.11.0

@lukesims
Copy link

You could specify an alias for your tasks with the task metadata.

displayName - When attached to a taskFunction creates an alias for the task. If using characters that aren't allowed in function names, use this property.

So you could use the following:

const testLocal = () => Promise.resolve();
testLocal.displayName = 'test-local';
export testLocal;

@coreyfarrell
Copy link
Member Author

I just tried this, it seems that displayName is not honored for tasks declared by ESM export. I tried the following:

const testLocal = () => Promise.resolve();
testLocal.displayName = 'test-local';
export {testLocal};

With this gulp testLocal works but gulp test-local does not.

The following does work (declares gulp test-local):

const testLocal = () => Promise.resolve();
testLocal.displayName = 'test-local';
gulp.task(testLocal);

@phated
Copy link
Member

phated commented Jan 30, 2019

We tried to support displayName with exported tasks but ran into the issue. See our revert and regression test at gulpjs/gulp-cli@672b8a8

If you could get it working correctly, it'd be an awesome addition to the CLI.

@phated
Copy link
Member

phated commented Feb 23, 2019

@coreyfarrell any interest in trying to make displayName work in the CLI?

@coreyfarrell
Copy link
Member Author

@phated I just did some troubleshooting, determined that undertaker is the source of the regression. gulpjs/undertaker#91 fixes the issue from that module. I'm not sure if this is considered breaking for undertaker, the docs do not really specify that displayName will be set by undertaker? That fix will allow lib/shared/register-exports.js to use gulpInst.task(task.displayName || taskName, task); without breaking the regression test. I tested by copying my undertaker changes to a clone of gulp-cli.

If/when the undertaker patch is accepted and released I can submit the fix for gulp-cli with an addition to the regression test for gulp.parallel.

export const something1 = gulp.series(a, b);
export const something2 = gulp.series(a, b);
something2.displayName = 'something-2';

With the current undertaker both exports have a displayName property, it's impossible for gulp-cli to tell that something2.displayName was overridden and that something1.displayName was not.

@phated
Copy link
Member

phated commented Apr 20, 2019

@coreyfarrell thanks for all the work here! I'm planning to release 2.2.0 of gulp-cli once travis/appveyor finish 😄

@phated
Copy link
Member

phated commented Apr 20, 2019

Published!

@coreyfarrell
Copy link
Member Author

That's great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment