awslabs / llrt

LLRT (Low Latency Runtime) is an experimental, lightweight JavaScript runtime designed to address the growing demand for fast and efficient Serverless applications.
Apache License 2.0
7.72k stars 341 forks source link

Issue with property named `get` inside classes #296

Open yspreen opened 3 months ago

yspreen commented 3 months ago

Express and Drizzle both have classes with property methods called get. It seems to be fine if defined like

get(/* args */) {
  /* method */
}

but this line in drizzle: https://github.com/drizzle-team/drizzle-orm/blob/main/drizzle-orm/src/sqlite-core/query-builders/delete.ts#L251

get: ReturnType<this['prepare']>['get'] = (placeholderValues) => {
    return this._prepare().get(placeholderValues);
};

causes issues in LLRT when compiled with TS and bundled with rollup.js:

invalid property name

because the compiled result is

get = (e) => this._prepare().get(e);
richarddavison commented 3 months ago

Hi @yspreen, thanks for the report. Do you have a minimum reproducible js file?

I can't reproduce this:

class Examle{
  get(){
      return "hello world")
  }
}

console.log(new Example().get())
yspreen commented 3 months ago
~/Downloads/llrt ./test.js
SyntaxError: invalid property name
    at ./test.js:2:6
class TestClass {
  get = () => console.log("get");
}

new TestClass().get();
yspreen commented 3 months ago

as mentioned above, it only breaks if you set it to an anonymous method. not if defining get() {}

richarddavison commented 2 months ago

I've located the issue and submitted a patch to downstream QuickJS engine. The catch is that we're not up to date on the latest version. However, we can (and already do) apply patches independently of the engine: https://github.com/bellard/quickjs/pull/258