Retain pure annotations in class fields #3599
Conversation
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via or load it into the REPL: |
Codecov Report
@@ Coverage Diff @@
## master #3599 +/- ##
=======================================
Coverage 96.35% 96.35%
=======================================
Files 180 180
Lines 6140 6145 +5
Branches 1799 1801 +2
=======================================
+ Hits 5916 5921 +5
Misses 111 111
Partials 113 113
Continue to review full report at Codecov.
|
src/utils/pureComments.ts
Outdated
| @@ -3,6 +3,14 @@ import * as acorn from 'acorn'; | |||
| import { base as basicWalker } from 'acorn-walk'; | |||
| import { CommentDescription } from '../Module'; | |||
|
|
|||
| // patch up acorn-walk until class-fields are officially supported | |||
| basicWalker.FieldDefinition = function (node: any, st: any, c: any) { | |||
| c(node.key, st, 'Expression'); | |||
There was a problem hiding this comment.
Shouldn't this line to be if ( node.computed ) { c(node.key, st, 'Expression'); }, just like MethodDefinition / Property do in acorn-work?
There was a problem hiding this comment.
Good catch, was obviously copying without thinking.
| @@ -0,0 +1,6 @@ | |||
| class main { | |||
| static k = /*#__PURE__*/ V(); | |||
| static /*#__PURE__*/ ignored; | |||
There was a problem hiding this comment.
Does this sample line has special meaning in rollup? It seems not a pure mark for function call.
Is it the new feature of rollup?
Or just as a test, to make sure here is no bug, without any other use?
There was a problem hiding this comment.
The latter, a test to cover a branch in the logic even if it makes no sense to do so in real code.
lukastaegert
force-pushed
the
gh-3591-class-field-annotations
branch
from
32e1295
to
49377f9
Compare
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Resolves #3591
Description
Unfortunately,
acorn-walkis not yet properly updated for the new nodes added for class fields. This adds a temporary patch until this is hopefully resolved once the feature is merged to acorn.