add class.hasInstance proposal (Stage 1) support

[YuriTu]

Q	A
Fixed Issues?	Darft https://github.com/tc39/proposal-class-brand-check
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR Link	proposal-class-brand-check es-tree
Any Dependency Changes?	No
License	MIT

Add TC39 proposal: proposal-class-brand-check

usage:

class Range {

  equals(range) {
    if (!(class.hasInstance(range))) return false;
  }
}

after transform :

var _set = new WeakSet();

class Range {
  constructor() {
    _set.add(this);
  }

  equals(range) {
    if (!_set.has(range)) return false;
  }

}

[JLHwung]

Can you open an PR to ESTree on the AST shape of class.hasInstance? See also contribution guide about new Babel plugins.

Since it is an early draft, we will not review until you mark it as ready. Take your time and happy hacking. If you have any questions, please reach out at Slack.

[JLHwung]

Left some comments on the parsing part. Will do another review on the transformer later.

Update: we should also take a look at the statement parsing:

babel/packages/babel-parser/src/parser/statement.js

Line 368 in 4820ea0

case tt._class:

class.hasInstance(foo) as an ExpressionStatement should be valid.

[JLHwung]

We should ensure @babel/generator prints the code according to the AST:

Suggested change

	export function ClassHasInstanceExpression(node: t.ClassHasInstanceExpression) {
	export function ClassHasInstanceExpression(node: t.ClassHasInstanceExpression) {
	this.word("class");
	this.token(".");
	this.word("hasInstance");
	this.printInnerComments(node);

Please also add a test case to babel-generator/test/fixtures/class-brand-check:

class Range {
    equals(obj) {
        class.hasInstance(obj)
        class/* 1 */./* 2 */hasInstance/* 3 */(obj)
    }
}

[nicolo-ribaudo]

We already have an AST node for meta properties:

babel/packages/babel-parser/src/types.js

Lines 887 to 891 in efe0188

	export type MetaProperty = NodeBase & {
	type: "MetaProperty",
	meta: Identifier,
	property: Identifier,
	};

We should keep using it instead of creating a new one. class.hasInstance(obj) would then be represented as a CallExpression whose callee is a MetaProperty, similarly to how we represent super() and import() as CallExpressions.

You can take inspiration from where we call the parseMetaProperty function.

[JLHwung]

The current AST spec follows ESTree proposal in estree/estree#260. The ClassHasInstanceExpression is similar to the ImportExpression in ESTree.

The ClassHasInstanceExpression approach is more abstract than the MetaProperty approach. So the advantages of each of them follows the ones of AST v.s. CST.

Pros of ClassHasInstanceExpression:

• The pattern is easier to detect than MetaProperty, which would require 4 checks: CallExpression, callee is MetaProperty, identifier is class and identifier is hasInstance
• AST constructed from the @babel/types ClassHasInstanceExpression constructor is always correct.

Pros of MetaProperty:

• Each non-punctuation token has corresponding syntax tree node so we don't need to deal with innerComments
• The current call expression Babel plugins will automatically apply to class.hasInstance() as well
• Better for parsing with recoverable errors: The syntax tree can represent more invalid AST, as long as accepted by the general CallExpression

If we are going with the MetaProperty approach, the pros of ClassHasInstanceExpression can be obtained from extra @babel/types helpers, but not vice versa.

[JLHwung]

With this change we will incorrectly allow

class class A {}

because class is eaten twice.

I suggest we remove this.takeDecorators in parseClassOrClassHasInstanceExpression and remove this.next() above.

[JLHwung]

The snapshot should be removed.

[JLHwung]

The output here is incorrect. It seems to me the comments are registered as innerComments of the BodyStatement, it should be attached as the innerComments of ClassHasInstanceExpression / MetaProperty.

[JLHwung]

The error message is a bit vague. We can provide more informative errors like we did for setters:

babel/packages/babel-parser/src/parse-error/standard-errors.js

Lines 48 to 51 in 2c7e9f7

	BadSetterArity: _("A 'set' accesor must have exactly one formal parameter."),
	BadSetterRestParameter: _(
	"A 'set' accesor function argument must not be a rest parameter.",
	),

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51691/

[nicolo-ribaudo]

According to https://github.com/tc39/notes/blob/HEAD/meetings/2021-01/jan-27.md#class-brand-checks and tc39/proposal-class-brand-check#15, it's likely that the syntax will change because:

• using a class. meta property conflicts with https://github.com/tc39/proposal-class-access-expressions, which is another stage 1 proposal
• some delegates don't like the hasInstance name because it doesn't reflect the goal of the proposal

Given those likely changes, I think it would be better to wait for the proposal champion to present the updated proposal version to TC39 before merging this PR.

[YuriTu]

This is valuable advice. I will discuss the plugin's plans with the authors of the proposal.

[liuxingbaoyu]

Thank you for your PR! Since this proposal is still in phase 1 and no longer seems to be active, I will close this PR.