GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Use TSX parser instead of Typescript parser for JavaScript and JSX by maxbrunsfeld · Pull Request #229 · github/semantic

This repository was archived by the owner on Apr 1, 2025. It is now read-only.

Conversation

@maxbrunsfeld

@maxbrunsfeld maxbrunsfeld commented Aug 14, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

JavaScript (and .jsx) files can contain JSX (and cannot use TypeScript's type assertion syntax), so we need to use the TSX dialect of TypeScript when parsing and analyzing them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
maxbrunsfeld added 2 commits August 14, 2019 10:08
@maxbrunsfeld
Use TSX parser instead of Typescript parser for JavaScript and JSX
233a51c
@maxbrunsfeld
Add comment about why JS and JSX are handled with the TSX parser
5d62300
@maxbrunsfeld

maxbrunsfeld commented Aug 14, 2019

Copy link
Copy Markdown
Contributor Author

Is there any testing that should be added for this? Previously, we would have gotten parse errors on any .js files that used JSX, so maybe just a unit test that we can parse a .js file with some JSX in it?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tclem

tclem commented Aug 14, 2019

Copy link
Copy Markdown
Member

Let's do something like this, but with a .jsx extension. You can use script/generate-example to help bootstrap the expected output.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
maxbrunsfeld and others added 2 commits August 21, 2019 10:31
@maxbrunsfeld Rick Winfrey
Handle JSX elements with type arguments
07ccbb8 
Also, add a unit test for JSX in JavaScript.

Co-Authored-By: Rick Winfrey <[email protected]>
@maxbrunsfeld
Regenerate jsx corpus files
5491001
@maxbrunsfeld

maxbrunsfeld commented Aug 21, 2019

Copy link
Copy Markdown
Contributor Author

While we were here, @rewinfrey and I also added some limited support for Type arguments in TSX (microsoft/TypeScript#22415).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
maxbrunsfeld
maxbrunsfeld commented Aug 21, 2019
View reviewed changes
Comment thread test/fixtures/tsx/corpus/jsx-elements.diffA-B.txt
(JsxElement
(JsxOpeningElement
(Identifier)
(Empty)

@maxbrunsfeld maxbrunsfeld Aug 21, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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.

JsxOpeningElement terms now have an optional TypeArguments child. We based this off the way that type arguments are handled in the New node.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
tclem
tclem approved these changes Aug 21, 2019
View reviewed changes

@tclem tclem left a comment

Copy link
Copy Markdown
Member

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.

LGTM. Can we do a manually run of the parse-examples target (we use to do this in CI, but travis can't quiet keep up with the workload)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@maxbrunsfeld
Merge branch 'master' into use-tsx-for-js-and-flow
73909f7
@maxbrunsfeld
maxbrunsfeld merged commit b7a52b4 into master Aug 21, 2019
@maxbrunsfeld
maxbrunsfeld deleted the use-tsx-for-js-and-flow branch August 21, 2019 18:44
@github github deleted a comment Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@tclem tclem tclem approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@maxbrunsfeld @tclem
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.