Split TypeScript assignment into TSX and TypeScript #102
Conversation
Also removes typeAssertion from TSX grammar
joshvera
left a comment
joshvera
left a comment
There was a problem hiding this comment.
Ready for review.
| super, | ||
| object, | ||
| array, | ||
| jsxElement', |
There was a problem hiding this comment.
I've had to duplicate typescript assignment in order to leave jsxElement' and jsxFragment in here. I could move assignment out into some Common module and pass in a list of terms to mappend instead. Thoughts?
There was a problem hiding this comment.
Thanks for calling out the duplication. Given the size of this patch as it is, I'd be in favor of merging this and to extract a common assignment module as a follow up PR. That might also make a weightier issue for someone in the community to take on. Either way I'm 👍
rewinfrey
left a comment
rewinfrey
left a comment
There was a problem hiding this comment.
👍 looks good to me! I left a couple questions.
| super, | ||
| object, | ||
| array, | ||
| jsxElement', |
There was a problem hiding this comment.
Thanks for calling out the duplication. Given the size of this patch as it is, I'd be in favor of merging this and to extract a common assignment module as a follow up PR. That might also make a weightier issue for someone in the community to take on. Either way I'm 👍
src/Language/TSX/Resolution.hs
Outdated
| typescriptExtensions = ["ts", "tsx", "d.ts"] | ||
|
|
||
| javascriptExtensions :: [String] | ||
| javascriptExtensions = ["js"] |
There was a problem hiding this comment.
Are there differences between this Resolution module and TypeScript.Resolution? I'm curious if we can still maintain a single Resolution module for TypeScript and TSX?
| , le "ruby" ".rb" "examples" (Just "script/known_failures.txt") | ||
| , le "typescript" ".ts" "examples" (Just "script/known_failures.txt") | ||
| , le "typescript" ".ts" "examples" (Just "typescript/script/known_failures.txt") | ||
| , le "typescript" ".tsx" "examples" (Just "typescript/script/known_failures.txt") |
There was a problem hiding this comment.
I'm not familiar with this context, but is "typescript" as the language correct here rather than "tsx"?
There was a problem hiding this comment.
Yeah "typescript" refers to the directory the example repos are in. This adds about 150 tests to parse-examples.
In tree-sitter/tree-sitter-typescript#68, we separated the TSX and TypeScript syntax in order to support type arguments in TSX and avoid ambiguity.
This PR adds a new assignment to semantic and moves
JSX,TSX, andJavaScriptto this new parser/assignment in order to assign more precisely. In practice this means type assertions are no longer supported in Flow/JS and JSX fragments are no longer parsed in TypeScript.