Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement optional chaining deletion #492

Merged
merged 1 commit into from
Dec 29, 2019

Conversation

alangpierce
Copy link
Owner

Progress toward #461

Tech plan: https://github.com/alangpierce/sucrase/wiki/Sucrase-Optional-Chaining-and-Nullish-Coalescing-Technical-Plan

This ended up being a relatively small tweak to the optional chaining
implementation:

  • There's a new helper that wraps the optionalChain helper, and I needed to add
    a way for helpers to depend on each other.
  • In most cases, we condition on the delete and non-delete cases based on
    whether there's a delete token just before the start of the optional chain.
  • To determine whether a subscript is the last of its chain (and therefore needs
    a delete operation), we can walk the tokens forward tracking depth until we
    get to either another subscript or the end of the chain. This means there was
    no need for any changes to the parser. Since it only walks between two
    adjacent subscripts, it takes linear extra time unless there's nesting, and only
    ever does the more expensive operation when processing an optional delete.

@alangpierce alangpierce force-pushed the implement-optional-chaining-delete branch from 2414adf to 08b28bb Compare December 29, 2019 02:37
@codecov-io
Copy link

codecov-io commented Dec 29, 2019

Codecov Report

Merging #492 into master will increase coverage by 0.14%.
The diff coverage is 92.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #492      +/-   ##
==========================================
+ Coverage   82.37%   82.52%   +0.14%     
==========================================
  Files          54       54              
  Lines        5276     5304      +28     
  Branches     1225     1236      +11     
==========================================
+ Hits         4346     4377      +31     
+ Misses        646      643       -3     
  Partials      284      284
Impacted Files Coverage Δ
src/TokenProcessor.ts 91.83% <100%> (+0.17%) ⬆️
src/HelperManager.ts 100% <100%> (ø) ⬆️
...transformers/OptionalChainingNullishTransformer.ts 89.58% <89.65%> (+4.39%) ⬆️
src/parser/traverser/expression.ts 87.73% <0%> (+0.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b475b8c...74371bc. Read the comment docs.

Progress toward #461

Tech plan: https://github.com/alangpierce/sucrase/wiki/Sucrase-Optional-Chaining-and-Nullish-Coalescing-Technical-Plan

This ended up being a relatively small tweak to the optional chaining
implementation:
* There's a new helper that wraps the optionalChain helper, and I needed to add
  a way for helpers to depend on each other.
* In most cases, we condition on the delete and non-delete cases based on
  whether there's a delete token just before the start of the optional chain.
* To determine whether a subscript is the last of its chain (and therefore needs
  a delete operation), we can walk the tokens forward tracking depth until we
  get to either another subscript or the end of the chain. This means there was
  no need for any changes to the parser. Since it only walks between two
adjacent subscripts, it takes linear extra time unless there's nesting, and only
ever does the more expensive operation when processing an optional delete.
@alangpierce alangpierce force-pushed the implement-optional-chaining-delete branch from 08b28bb to 74371bc Compare December 29, 2019 02:52
@alangpierce alangpierce merged commit 8b7a4f7 into master Dec 29, 2019
@alangpierce alangpierce deleted the implement-optional-chaining-delete branch December 29, 2019 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants