fix(js_formatter): don't insert trailing comma on type import statement #4454
Conversation
github-actions
bot
added
A-Parser
xtask/codegen/js.ungram
Outdated
| // a: import("./test").T<typeof X> | ||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| // a: import("./test")<string> | ||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| TsImportType = | ||
| 'typeof'? | ||
| 'import' | ||
| arguments: JsCallArguments | ||
| arguments: TsImportArguments |
There was a problem hiding this comment.
Hey, based on the typescript source, it looks like it should be like this:
// (not tested)
TsImportType =
'typeof'?
'import'
argument: AnyTsType
(
','
'{'
('assert' | 'with')
':'
'{'
assertions: TsImportTypeAssertionList
'}'
'}'
)?
qualifier_clause: TsImportTypeQualifier?
type_arguments: TsTypeArguments?
TsImportTypeAssertionList =
(TsImportTypeAssertion (',' TsImportTypeAssertion)* ','?)
TsImportTypeAssertion =
key: ('identifier' | 'js_string_literal')
':'
value: AnyJsExpression // parse_assignment_expression_or_higher
https://github.com/microsoft/TypeScript/blob/main/src/compiler/parser.ts#L4546
- The first argument is any TS type
- (not only a string - the "String literal expected" error comes from the typechecker)
- The
,{assert:{and}}are hardcoded tokens, it's not a comma separated list - The part inside the inner braces is a comma separated list of
key:expressionwhere key can only be an identifier or string literal, and expression is a JS expression (parseAssignmentExpressionOrHigher).- Trailing commas are allowed
Examples:
type A = typeof import(1); // ok (typechecker error)
type B = typeof import("a.json",{}); // syntax error - missing assert keyword
type C = typeof import("a.json",{assert:{}}); // ok
type D = typeof import("a.json",{assert:{a:"1"}}); // ok
type E = typeof import("a.json",{assert:{[a]:"1"}}); // syntax error - keys need to be identifiers or string literals
type F = typeof import("a.json",{assert:{"a":"1"}}); // ok
type G = typeof import("a.json",{assert:{a:"1", b:"2",}}); // ok
type H = typeof import("a.json",{assert:{}}, 1); // syntax error - 3rd arg not valid
type I = typeof import(import(Record<string,string>),{assert:{a:await p}}); // ok (typechecker error)
There was a problem hiding this comment.
😯, thank you very much to provide the typescript context, i will base this on to update my code here, I haven't seen the relevant parser specification reference for this before.
fireairforce
force-pushed
the
fix-4334
branch
from
883ca65
to
206a8ef
Compare
| // test ts ts_import_type_arguments | ||
| // type A = import("foo"); | ||
| // type B = import("foo", "bar"); | ||
| // type C = typeof import("foo"); |
There was a problem hiding this comment.
We moved away from this testing. Now just create a file inside the specs and run cargo t to generate the snapshot
There was a problem hiding this comment.
lol, I asked why I panic here when I execute cargo codegen test recently. i will update later~ now the parser js test case is align with css parser.
There was a problem hiding this comment.
Exactly. The comments in the source code are still there because the PR was too big. We will have to delete them eventually
Summary
closes: #4334
Test Plan