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
[tsx] raise error on single arrow type argument without comma #14135
Changes from 11 commits
a7fd060
0b0d2e9
117e71d
aa29eb1
c22c322
4621b27
c7d16af
a0df5f9
07083a8
c396454
6a2e55d
15059cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
// Same as `generic`. Verify that JSX doesn't change things. | ||
<T>(a: T): T => a; | ||
<T,>(a: T): T => a; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ import { | |
} from "../../tokenizer/types"; | ||
import { types as tc } from "../../tokenizer/context"; | ||
import * as N from "../../types"; | ||
import type { Position } from "../../util/location"; | ||
import { Position, createPositionWithColumnOffset } from "../../util/location"; | ||
import type Parser from "../../parser"; | ||
import { | ||
type BindingTypes, | ||
|
@@ -148,6 +148,8 @@ const TSErrors = makeErrorTemplates( | |
"A 'set' accessor cannot have rest parameter.", | ||
SetAccesorCannotHaveReturnType: | ||
"A 'set' accessor cannot have a return type annotation.", | ||
SingleTypeParameterWithoutTrailingComma: | ||
"Single type parameter %0 should have a trailing comma. Example usage: <%0,>.", | ||
StaticBlockCannotHaveModifier: | ||
"Static class blocks cannot have any modifier.", | ||
TypeAnnotationAfterAssign: | ||
|
@@ -2933,6 +2935,7 @@ export default (superClass: Class<Parser>): Class<Parser> => | |
// Either way, we're looking at a '<': tt.jsxTagStart or relational. | ||
|
||
let typeParameters: ?N.TsTypeParameterDeclaration; | ||
let invalidSingleType: ?N.TsTypeParameter; | ||
state = state || this.state.clone(); | ||
|
||
const arrow = this.tryParse(abort => { | ||
|
@@ -2952,9 +2955,36 @@ export default (superClass: Class<Parser>): Class<Parser> => | |
this.resetStartLocationFromNode(expr, typeParameters); | ||
} | ||
expr.typeParameters = typeParameters; | ||
|
||
// report error if single type parameter used without trailing comma. | ||
if ( | ||
this.hasPlugin("jsx") && | ||
expr.typeParameters.params.length === 1 && | ||
!expr.typeParameters.extra?.trailingComma | ||
) { | ||
const parameter = expr.typeParameters.params[0]; | ||
if (parameter.constraint) { | ||
// If parameter has any constraints, it must contain multiple tokens. | ||
// <T extends U> is a valid declaration. | ||
// <T extends {name: string}> is also a valid declaration. | ||
} else invalidSingleType = parameter; | ||
nicolo-ribaudo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return expr; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you move the check before this line? It should simplify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. of course |
||
}, state); | ||
|
||
if (invalidSingleType) { | ||
this.raise( | ||
TSErrors.SingleTypeParameterWithoutTrailingComma, | ||
{ | ||
at: createPositionWithColumnOffset(invalidSingleType.loc.end, 1), | ||
}, | ||
process.env.BABEL_8_BREAKING | ||
? invalidSingleType.name.name | ||
: invalidSingleType.name, | ||
); | ||
} | ||
|
||
/*:: invariant(arrow.node != null) */ | ||
if (!arrow.error && !arrow.aborted) { | ||
// This error is reported outside of the this.tryParse call so that | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
// Same as `generic`. Verify that JSX doesn't change things. | ||
<T>(a: T): T => a; | ||
<T,>(a: T): T => a; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to check it only in tsx file? @ozanhonamlioglu
(#14361 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolo-ribaudo Sounds logical to me, shall I change it with tsx? What is your opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TSX is "jsx in TypeScript". This check is only executed when TypeScript is enabled, so it's already only checking for tsx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our project, we used the same presets (
@babel/preset-env
+@babel/preset-react
+@babel/preset-typescript
) to transform all the files, which caused the.ts
files to be processed by the jsx plugin, so the generics in the.ts
files also triggered theSingleTypeParameterWithoutTrailingComma
error.This is a wrong usage, we will improve it. Thank you! @ozanhonamlioglu @nicolo-ribaudo