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

refactor(rome_js_parser): Inroduce TypeContext #4123

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

MichaReiser
Copy link
Contributor

@MichaReiser MichaReiser commented Dec 30, 2022

Summary

This PR moves the state tracking if conditional types are allowed from the ParserState into a new TypeContext explicitly passed as an argument to type parsing methods, similar to the ExpressionContext for parsing expressions.

The advantage of an passing the context as an argument over an additional flag on the state is that it is easier to reason about the state of the parser and avoids accidental "leaking" of the conditional types when switching between parsing types and expressions (as it is the case when parsing the function parameters).

Test Plan

I verified that the fixed regressions are intentional.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Dec 30, 2022

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit daa804b
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63aede70799d7200091a688e

@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48647 48647 0
Passed 47582 47582 0
Failed 1065 1065 0
Panics 0 0 0
Coverage 97.81% 97.81% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6093 6093 0
Passed 1754 1754 0
Failed 4339 4339 0
Panics 0 0 0
Coverage 28.79% 28.79% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 565 567 ✅ ⏫ +2
Failed 74 72 ✅ ⏬ -2
Panics 0 0 0
Coverage 88.42% 88.73% +0.31%
🎉 Fixed (2):
regression/nested-extends-in-arrow-type-param
regression/nested-extends-in-arrow-type-param-babel-7

ts/microsoft

Test result main count This PR count Difference
Total 16740 16740 0
Passed 12816 12816 0
Failed 3924 3924 0
Panics 0 0 0
Coverage 76.56% 76.56% 0.00%

@MichaReiser
Copy link
Contributor Author

!bench_parser

@github-actions
Copy link

Parser Benchmark Results

group                                 main                                   pr
-----                                 ----                                   --
parser/big5-added.json                1.00   211.9±13.47µs    79.7 MB/sec    1.03   217.6±10.53µs    77.7 MB/sec
parser/canada.json                    1.00    108.5±9.31ms    19.8 MB/sec    1.03   111.2±11.25ms    19.3 MB/sec
parser/checker.ts                     1.00    136.3±8.60ms    19.1 MB/sec    1.07    146.1±6.04ms    17.8 MB/sec
parser/compiler.js                    1.00     75.7±5.72ms    13.8 MB/sec    1.02     77.5±6.09ms    13.5 MB/sec
parser/d3.min.js                      1.00     44.4±2.57ms     5.9 MB/sec    1.02     45.2±2.92ms     5.8 MB/sec
parser/db.json                        1.00      5.6±0.35ms    31.8 MB/sec    1.00      5.6±0.29ms    31.9 MB/sec
parser/dojo.js                        1.02      4.0±0.19ms    17.3 MB/sec    1.00      3.9±0.19ms    17.6 MB/sec
parser/eucjp.json                     1.00   350.0±18.16µs   111.9 MB/sec    1.05   366.4±17.01µs   106.9 MB/sec
parser/ios.d.ts                       1.01    123.8±6.66ms    15.1 MB/sec    1.00    122.0±8.52ms    15.3 MB/sec
parser/jquery.min.js                  1.02     12.4±0.81ms     6.7 MB/sec    1.00     12.1±0.64ms     6.8 MB/sec
parser/math.js                        1.02     94.3±6.97ms     6.9 MB/sec    1.00     92.1±6.27ms     7.0 MB/sec
parser/package-lock.json              1.00      2.3±0.16ms    60.3 MB/sec    1.02      2.3±0.11ms    59.2 MB/sec
parser/parser.ts                      1.00      2.8±0.11ms    17.5 MB/sec    1.05      2.9±0.11ms    16.6 MB/sec
parser/pixi.min.js                    1.01     56.9±4.53ms     7.7 MB/sec    1.00     56.5±4.42ms     7.8 MB/sec
parser/react-dom.production.min.js    1.02     16.6±0.83ms     6.9 MB/sec    1.00     16.3±0.93ms     7.1 MB/sec
parser/react.production.min.js        1.00   840.2±40.80µs     7.3 MB/sec    1.03   866.1±53.34µs     7.1 MB/sec
parser/router.ts                      1.00  1124.0±71.44µs    27.4 MB/sec    1.03  1156.2±82.03µs    26.6 MB/sec
parser/tex-chtml-full.js              1.00    131.6±9.16ms     6.9 MB/sec    1.00    131.0±7.70ms     7.0 MB/sec
parser/three.min.js                   1.00     62.1±3.96ms     9.5 MB/sec    1.12     69.8±4.67ms     8.4 MB/sec
parser/typescript.js                  1.01   573.3±20.86ms    16.6 MB/sec    1.00   565.1±21.97ms    16.8 MB/sec
parser/vue.global.prod.js             1.00     19.6±1.11ms     6.1 MB/sec    1.03     20.3±1.25ms     5.9 MB/sec

@MichaReiser MichaReiser added L-JavaScript Langauge: JavaScript A-Parser Area: parser labels Jan 3, 2023
@MichaReiser MichaReiser modified the milestone: Next Jan 3, 2023
@MichaReiser MichaReiser marked this pull request as ready for review January 3, 2023 10:14
@ematipico ematipico merged commit d403cde into main Jan 4, 2023
@ematipico ematipico deleted the refactor/type-context branch January 4, 2023 09:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Parser Area: parser L-JavaScript Langauge: JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants