Export anonymous functions in 2 steps, declare as variable and then assign to exports. #39820
Conversation
This allows the browser or node to properly name the (arrow) function
typescript-bot
added
the
For Milestone Bug
|
|
josejulio
changed the title
|
One thing that noticed, is that the comments are both set on I found a way to remove them, but not sure if is the best. |
| var req; | ||
| return __generator(this, function (_a) { | ||
| switch (_a.label) { | ||
| case 0: return [4 /*yield*/, new Promise(function (resolve_5, reject_5) { require(['./test'], resolve_5, reject_5); })]; // FIVE | ||
| case 0: return [4 /*yield*/, import('./test')]; // FIVE |
There was a problem hiding this comment.
I didn't notice this before, but doesn't look good.
There was a problem hiding this comment.
possibly missing a call to visitNode somewhere.
There was a problem hiding this comment.
I fixed this with: 984e304
There was a problem hiding this comment.
but yes, a call to visitNode was missing :)
| @@ -1206,7 +1207,31 @@ namespace ts { | |||
| variables = append(variables, variable); | |||
| } | |||
| else if (variable.initializer) { | |||
| expressions = append(expressions, transformInitializedVariable(variable as InitializedVariableDeclaration)); | |||
| if (!isBindingPattern(variable.name) && [SyntaxKind.ArrowFunction, SyntaxKind.FunctionExpression].includes(variable.initializer.kind)) { | |||
There was a problem hiding this comment.
Array.prototype.includes isn't guaranteed safe to use as the language service can still run in ES5 engines that don't include this method. It would be better to just use isArrowFunction(variable.initializer) || isFunctionExpression(variable.initializer) or compare the kinds directly to avoid allocating and throwing away a 2-element array object for a simple comparison.
There was a problem hiding this comment.
We target ES5, but we unintentionally get ES2015+ features showing up as available due to the fact that @types/node is a devDependency.
There was a problem hiding this comment.
Sounds good, I'll update it to use isArrowFunction/isFunctionExpression.
- Use isArrowFunction and isFunctionExpression
| @@ -1206,7 +1207,31 @@ namespace ts { | |||
| variables = append(variables, variable); | |||
| } | |||
| else if (variable.initializer) { | |||
| expressions = append(expressions, transformInitializedVariable(variable as InitializedVariableDeclaration)); | |||
| if (!isBindingPattern(variable.name) && (isArrowFunction(variable.initializer) || isFunctionExpression(variable.initializer))) { | |||
There was a problem hiding this comment.
Sorry I missed this before, but shouldn't class expressions be included in this as well? They can also be named based on the variable:
export const C = class {};There was a problem hiding this comment.
No worries, I forgot about them.
I'll update to consider them too.
- Consider ClassExpresion, they can also be named based on the variable.
|
Since the issue for this is marked for 4.1, I have to wait to merge this until tomorrow at the earliest. |
|
@rbuckton wonder if this can be merged already now that there is a release-4.0 branch. Thanks! |
|
Thanks for the reminder, this has been merged :) |
Fixes #39819