Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#rollup-class-fields

or load it into the REPL:
https://rollupjs.org/repl/?circleci=10460

Codecov Report

Merging #3488 into master will increase coverage by 0.10%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #3488      +/-   ##
==========================================
+ Coverage   95.96%   96.06%   +0.10%     
==========================================
  Files         174      175       +1     
  Lines        5947     5950       +3     
  Branches     1752     1752              
==========================================
+ Hits         5707     5716       +9     
+ Misses        123      118       -5     
+ Partials      117      116       -1     

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 7abfb93...86843d1. Read the comment docs.

On second thought, maybe I want to at least add the new AST node types for class properties...

Ok, Nodes are added.

I do not pretend to understand this magic, but it looks great! I suppose we can remove the WIP status of this PR then?

Should we test static expressions and static functions as well here?

*private static methods I mean

I suppose we can remove the WIP status of this PR then?

Definitely

Should we test static expressions and static functions as well here?

Please feel free to add this, I must admit I was not entirely sure what was included. I will not be able to look at this again before tonight.

@guybedford I extended the test to include all new features and, who would have thought, support is incomplete: Private methods, setters and getters produce an "unexpected token" while parsing. I assume this needs to be fixed in the plugin. Should we still release it as "incomplete support"? It still provides some value and also fixes a bug.

You can remove the comments in my test case to see what I mean.

@lukastaegert thankfully, for now at least, I believe this syntax support level exactly matches Chrome stable (and hence the major usage). But yes as soon as Chrome lands those that will be an issue. Perhaps maintaining and developing the forks as part of Rollup will be an option here.

Note that also eg Terser doesn't support any of this yet (I know @fabiosantoscode was originally against it)... perhaps this is another area where RollupJS could assist with funding for these features.

You are right, Chrome does not support any of the missing ones. It also does not support static private methods, so there is at least one additional case that is supported. Then I will push this one out as well.

Private methods and accessors, both static and instance, are implemented in V8 and are unflagged now in Chrome 84 as well. Will Rollup be supporting these as well, and if so, should an issue be opened to track that?

I doubt it’s really this simple, but FWIW, in this code —

    // Parse private static methods
    parsePropertyName(prop) {
      if (prop.static && this.type == this.privateNameToken) {
        this.parsePrivateClassElementName(prop);
      } else {
        super.parsePropertyName(prop);
      }
    }

If I remove prop.static from the condition, unexpected token errors from instance private methods stop and the output looks correct.

You can create an issue, but this is not Rollup code, without an upstream issue in the corresponding acorn plugin (likely https://github.com/acornjs/acorn-private-class-elements), nothing will move forward here.

Cool — in that case, it seems like maybe Rollup support (at least for instance methods) might end up being “free” if the upstream plugin is updated to handle these cases. Will investigate a bit further, thanks.

Yes, that is my expectation. Others will profit from this as well.

Hey there!

I was against class fields, but I wouldn't stop progress for just that :) they are implemented in Terser. I don't recall if I got to implementing private fields too, though.