Added transform for logical assignment operators #557
Conversation
Codecov Report
@@ Coverage Diff @@
## master #557 +/- ##
==========================================
+ Coverage 81.47% 81.59% +0.12%
==========================================
Files 55 56 +1
Lines 5538 5624 +86
Branches 1308 1325 +17
==========================================
+ Hits 4512 4589 +77
- Misses 735 739 +4
- Partials 291 296 +5
Continue to review full report at Codecov.
|
|
|
||
| export default class LogicalAssignmentTransformer extends Transformer { | ||
| // This stack stores the state needed to transform computed property access | ||
| private readonly computedAccessStack: Array<ComputedAccess> = []; |
There was a problem hiding this comment.
Switched to a manual stack instead of relying on the JS execution stack. This seemed to avoid some of the competition going on with other processors, but could switch back if you think the old approach is better.
|
@alangpierce Sorry for the ping, but any chance of getting this published? :) Upon testing, it works (yay!) but outputs some questionable stuff. _logicalAssign(type, 'properties', '??=', () => {});Is this really necessary? foo ??= bar;
// can be transformed as
if (foo == null) foo = bar;I don't see why it requires a function call, string comparison, etc. just to do a simple assignment. |
|
@Qix- #550 (comment) has more context on why the transform is the way it is. Also note that the helper transform is avoided when the left side is a plain variable. Could be discussed if it's worth merging this btw, as we're approaching support for the syntax in all major runtimes. |
|
Gonna close this since Node.js v16 is the first LTS release with support for this syntax and it is soon the active LTS. At this point it doesn't make sense to add the complexity of this transform to sucrase, with it being better to rather look at which transforms we can remove. |
|
I don't use Sucrase for Node. I use it for the browser. There are recent browsers that still don't support this syntax. |
Implements transforms of
&&=,||=, and??=, as discussed in #550.This is an early draft to see what you think about the direction. It does end up adding more fields to the tokens, but it's also real tricky to get around that. I did end up with some thoughts about how to refactor into a more generic helper prefixing field, but that'd be a separate thing.
Only tested with transform tests so far, haven't actually executed any transformed code, so needs some more testing there x)