Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feature(rome_js_parser): JSX support in .js files #2674

Merged
merged 1 commit into from
Jun 14, 2022

Conversation

NicholasLYang
Copy link
Contributor

@NicholasLYang NicholasLYang commented Jun 7, 2022

Summary

Addresses #2625. Because TypeScript allows JSX in .js files by default, we allow the same. Also makes adoption easier because many users have React components in a .js file.

Test Plan

The existing tests for JSX should suffice.

@NicholasLYang NicholasLYang temporarily deployed to aws June 7, 2022 16:49 Inactive
@github-actions
Copy link

github-actions bot commented Jun 7, 2022

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45878 45878 0
Passed 44938 44938 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 350 350 0
Failed 5596 5596 0
Panics 0 0 0
Coverage 5.89% 5.89% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12391 12393 ✅ ⏫ +2
Failed 3866 3864 ✅ ⏬ -2
Panics 0 0 0
Coverage 76.22% 76.23% +0.01%
🎉 Fixed (2):
compiler/jsxPreserveWithJsInput.ts
compiler/reactImportDropped.ts

@github-actions
Copy link

github-actions bot commented Jun 7, 2022

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jun 7, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 17a117b
Status: ✅  Deploy successful!
Preview URL: https://f66221df.tools-8rn.pages.dev
Branch Preview URL: https://fix-jsx-in-js-files.tools-8rn.pages.dev

View logs

@NicholasLYang NicholasLYang temporarily deployed to aws June 7, 2022 18:58 Inactive
@NicholasLYang NicholasLYang marked this pull request as ready for review June 7, 2022 19:23
@NicholasLYang NicholasLYang temporarily deployed to aws June 7, 2022 19:58 Inactive
crates/rome_analyze/tests/specs/flipBinExp.js Show resolved Hide resolved
crates/rome_js_parser/src/syntax/jsx/mod.rs Outdated Show resolved Hide resolved
@@ -34,12 +34,12 @@ use super::typescript::parse_ts_type_arguments;
// test jsx jsx_element_as_statements
// <div />

// test_err jsx_or_type_assertion
// test_err cjs jsx_or_type_assertion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we verify if typescript enables JSX for cjs too? I don't see why it should be disabled, considering that server rendering is a thing now with react.

You can probably use

// SCRIPT
// ... test code

To force run the test with JSX disabled and in script mode

value_token: IDENT@28..31 "div" [] [],
},
type_arguments: missing (optional),
expression: JsxTagExpression {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to still be parsed out as JSX. Can we change the test back to be parsed with JSX disabled (so that the snapshot doesn't change)

@MichaReiser
Copy link
Contributor

!bench_parser

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Parser Benchmark Results

group                                 main                                   pr
-----                                 ----                                   --
parser/checker.ts                     1.00    171.4±9.97ms    15.2 MB/sec    1.00    171.2±7.23ms    15.2 MB/sec
parser/compiler.js                    1.00    103.4±4.70ms    10.1 MB/sec    1.01    104.8±6.00ms    10.0 MB/sec
parser/d3.min.js                      1.00     61.5±2.85ms     4.3 MB/sec    1.02     63.0±3.31ms     4.2 MB/sec
parser/dojo.js                        1.00      5.6±0.25ms    12.3 MB/sec    1.03      5.8±0.22ms    11.9 MB/sec
parser/ios.d.ts                       1.00    143.1±4.93ms    13.0 MB/sec    1.01    144.8±6.52ms    12.9 MB/sec
parser/jquery.min.js                  1.00     16.5±0.63ms     5.0 MB/sec    1.02     16.9±0.66ms     4.9 MB/sec
parser/math.js                        1.05    127.6±6.69ms     5.1 MB/sec    1.00    121.9±5.71ms     5.3 MB/sec
parser/parser.ts                      1.00      3.9±0.13ms    12.3 MB/sec    1.00      3.9±0.14ms    12.3 MB/sec
parser/pixi.min.js                    1.00     73.7±3.78ms     6.0 MB/sec    1.02     75.2±4.22ms     5.8 MB/sec
parser/react-dom.production.min.js    1.01     22.9±0.99ms     5.0 MB/sec    1.00     22.6±0.75ms     5.1 MB/sec
parser/react.production.min.js        1.03  1278.9±66.36µs     4.8 MB/sec    1.00  1245.7±45.41µs     4.9 MB/sec
parser/router.ts                      1.02      3.2±0.13ms    18.9 MB/sec    1.00      3.2±0.12ms    19.3 MB/sec
parser/tex-chtml-full.js              1.00    162.1±7.66ms     5.6 MB/sec    1.00    162.6±5.81ms     5.6 MB/sec
parser/three.min.js                   1.00     84.0±4.01ms     7.0 MB/sec    1.01     84.6±4.50ms     6.9 MB/sec
parser/typescript.js                  1.03   718.8±31.82ms    13.2 MB/sec    1.00   700.2±24.57ms    13.6 MB/sec
parser/vue.global.prod.js             1.00     28.2±1.12ms     4.3 MB/sec    1.03     29.0±1.27ms     4.2 MB/sec

@NicholasLYang NicholasLYang temporarily deployed to aws June 8, 2022 17:10 Inactive
@MichaReiser MichaReiser self-assigned this Jun 14, 2022
@MichaReiser MichaReiser linked an issue Jun 14, 2022 that may be closed by this pull request
@MichaReiser MichaReiser temporarily deployed to aws June 14, 2022 06:39 Inactive
Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I address the review feedback. We should be good to merge now

@MichaReiser MichaReiser changed the title feature(parser): JSX support in .js files feature(rome_js_parser): JSX support in .js files Jun 14, 2022
@MichaReiser MichaReiser merged commit 89641b4 into main Jun 14, 2022
@MichaReiser MichaReiser deleted the fix/jsx-in-js-files branch June 14, 2022 06:46
IWANABETHATGUY pushed a commit to IWANABETHATGUY/tools that referenced this pull request Jun 15, 2022
Enable JSX by default for all javascript files (not typescript)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

📎 Allow JSX in .js files
2 participants