Support private fields for classes with #-prefix

[ArneBab]

private fields in Javascript classes (https://github.com/tc39/proposal-class-fields) are currently shown as errors.

It would be great if js2 could treat them as supported syntax.

Draft for a test:

(js2-deftest-parse parse-class-private-field-with-init
  "class C {\n  #x = 42;\n  #y = 24;\n}"
  :reference "class C {\n  #x = 42\n  #y = 24\n}")

We already use them via babel, and I get the error "invalid property id", and I don’t understand where js2-name-node-p is defined.

[ArneBab]

(defun js2-identifier-start-p (c)
  "Is C a valid start to an ES5 Identifier?
See http://es5.github.io/#x7.6"
  (or
   (memq c '(?$ ?_ ?#)) ;; needs the ?# added here for private fields in classes
   (memq (get-char-code-property c 'general-category)
         ;; Letters
         '(Lu Ll Lt Lm Lo Nl))))

(means that I now know where js2-name-node-p is defined :-) )

For better undefined-warning support, this still needs to understand this.#something = syntax, but the above is already a solid improvement.

[ArneBab]

See #541

[dgutov]

I'd rather not merge unfinished features, they're likely to trigger further bug reports anyway.

If you really need this for your work and you don't have the time to write a complete support, you can apply this in your own config using advice-add.

[ArneBab]

How can I use advice-add for this? (for my own use I redefined the function)

The feature is stage 3 and there are polyfills, Chrome and Babel already support the feature: https://github.com/tc39/proposal-class-fields#implementations

Also there is support in IntelliJ and Sublime for this, but I really don’t want to use IntelliJ for Javascript, since js2-mode provides comparable JS-support in Emacs (where it is much more comfortable to work): tc39/proposal-class-fields#57

[olanod]

Any pointer on how to add support for it at least as a local hack? Looks mature enough, I'm using it in Chrome already with no transpiling, the highlighting is very ugly and confusing at the moment :(

[ArneBab]

I created a pull-request that adds basic support: #541

There’s still stuff missing (like recognizing this.#foo = 1 as assignment to the field), but at least it does not give errors anymore.

[dgutov]

@ArneBab Could you clarify how this.#foo = 1 is recognized instead?

[dgutov]

Here's the "local hack" version:

(advice-add #'js2-identifier-start-p
            :after-until
            (lambda (c) (eq c ?#)))

[ArneBab]

I use this as local hack:

(use-package js2-mode :ensure t :defer 20
  :mode
  (("\\.js\\'" . js2-mode))
  :config
  (setq js-indent-level 2)
  ;; patch in basic private field support
  (defun js2-identifier-start-p (c)
  "Is C a valid start to an ES5 Identifier?
See http://es5.github.io/#x7.6"
  (or
   (memq c '(?$ ?_ ?#)) ;; adds ?#
   (memq (get-char-code-property c 'general-category)
         ;; Letters
         '(Lu Ll Lt Lm Lo Nl))))
  (defun js2-identifier-part-p (c)
  "Is C a valid part of an ES5 Identifier?
See http://es5.github.io/#x7.6"
  (or
   (memq c '(?$ ?_ ?\u200c  ?\u200d ?#))
   (memq (get-char-code-property c 'general-category)
         '(;; Letters
           Lu Ll Lt Lm Lo Nl
           ;; Combining Marks
           Mn Mc
           ;; Digits
           Nd
           ;; Connector Punctuation
           Pc)))))

[dgutov]

@ArneBab Does my version work for you?

If it does, please don't advertise the more brittle approach.

[ArneBab]

Does your approach work for js2-identifier-start-p, too?
Or is that part unnecessary?

[dgutov]

What do you mean? It's exactly the function I applied the advice to.

If you mean -part-p, then a) it's not in your PR, b) I don't think it's needed, no.

[ArneBab]

the -part-p is why I posted this here, because it is not in my PR, but I had to apply it locally to cope with this.#foo.

[dgutov]

My approach would work with it, but what example exactly does need that change?

[ArneBab]

I got the problem when working on a codebase that assigned to this.#foo and later used this.#foo. If it isn’t needed I might have been in a bad state (i.e. the patching might not have been used).

As late reply to this.#foo = 1: If I understand it correctly, this.#foo is recognized as its own variable, but does not mark #foo as used.

[dgutov]

I got the problem when working on a codebase that assigned to this.#foo and later used this.#foo

I don't see it.

[ArneBab]

I’ll try the advice-approach and see whether I run into problems again.

[ArneBab]

yes, your approach also works.

I get warnings about undeclared variables, but no parse-errors.

[dgutov]

This should work more strictly than the advice posted previously. Still could be stricter (like checks against super.#abc or using private fields outside classes), PRs welcome.

@ArneBab Could you check that the new support at least allows all valid cases? I've tested a bunch, but could miss something.