quickjs icon indicating copy to clipboard operation
quickjs copied to clipboard

Class method with name `get` throws incorrect "invalid property name" exception

Open richarddavison opened this issue 1 year ago • 4 comments

When using arrow functions to define methods on a class, QJS throws exception:

SyntaxError: invalid property name
    at ./test.js:2:6

class TestClass {
  get = () => console.log("get");
}

new TestClass().get();

richarddavison avatar Apr 02 '24 09:04 richarddavison

I think this might be a bigger issue here, we can't have any property — be it on a class or an object — that has a name for a reserved keyword. This doesn't seem to be the behavior of other engines, though, since Node and the browsers I've tested with don't really care.

https://github.com/bellard/quickjs/blob/3b45d155c77bbdfe9177b1e03db830d2aff0b2a8/quickjs.c#L22385-L22400

I think it should probably not check for reserved keywords like this, since the spec doesn't really mention anything on this.


Might be a bit troublesome to deal with stuff like this though image

gabrielmfern avatar Apr 10 '24 16:04 gabrielmfern

I just tried this:

class TestClass { "get" = () => console.log("get"); }

And it works. Property names can be identifiers, numbers, strings or computed property names (expressions between [ and ]).

Now the question is should get be rejected as a syntax error? Probably not and we should fix this. We already accept keywords as class property names, but get is a special case as this syntax is supported:

class TestClass2 { get x() { console.log("get x"); return 1; }}

So we should accept get as a property name if followed by a = or ;. Same for set.

chqrlie avatar Apr 10 '24 16:04 chqrlie

@chqrlie Are you also considering allowing a class method to have a name of a reserved keyword? I'm facing a compatibility issue that happens because:

class Example {
  static() { return 1234 }
}

is not supported on quickjs

gabrielmfern avatar Apr 11 '24 13:04 gabrielmfern

@gabrielmfern: yes, the fix should also allow this particular case.

chqrlie avatar Apr 11 '24 14:04 chqrlie