Chevrotain / chevrotain

Parser Building Toolkit for JavaScript
https://chevrotain.io
Apache License 2.0
2.44k stars 200 forks source link

runtime exception: `Class constructor GAstVisitor cannot be invoked without 'new'` #1845

Closed mshima closed 1 year ago

mshima commented 1 year ago
ERROR! Class constructor GAstVisitor cannot be invoked without 'new'
TypeError: Class constructor GAstVisitor cannot be invoked without 'new'
    at new DslMethodsCollectorVisitor (/Users/mshima/aplicacoes/default/node_modules/chevrotain/lib/src/parse/parser/traits/looksahead.js:107:47)
    at Object.<anonymous> (/Users/mshima/aplicacoes/default/node_modules/chevrotain/lib/src/parse/parser/traits/looksahead.js:148:24)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/Users/mshima/aplicacoes/default/node_modules/chevrotain/lib/src/parse/parser/parser.js:34:20)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)

Reproducing:

npx generator-jhipster@latest jdl bug-tracker.jh

Related to https://github.com/jhipster/generator-jhipster/pull/19483

bd82 commented 1 year ago

Hello @mshima

I'm not seeing any issues in a small example consuming latest (10.2.0) chevrotain via commonjs. Nor am I spotting anything (e.g cyclic dependencies related to GAstVisitor) by looking at the code.

Also the latest tag of generator-jhipster is 7.9.2 and this seems to be using 10.1.2 (locked) version of Chevrotain. So I'm not sure how the update to 10.2.0 is related to breakage in 7.9.2 (latest)...

Can you provide a way for me to debug this?

mshima commented 1 year ago

You can install jhipster locally and run it.

npm install generator-jhipster@7.9.2
npx jhipster jdl bug-tracker.jh

Updating to chevrotain@10.2.0 fails too: https://github.com/jhipster/generator-jhipster/pull/19487

git clone https://github.com/jhipster/generator-jhipster.git
cd generator-jhipster
npm install
npm install chevrotain@latest
npx mocha test/jdl --no-parallel # test/jdl is where chevrotain is used.

This is a big issue for jhipster since it breaks every new installation. https://github.com/jhipster/generator-jhipster/issues/19486

I probably won't manage to debug this today.

bd82 commented 1 year ago

I wonder if this is somehow related to how npx runs things. Because with yeoman directly it works properly:

{
  "name": "blah",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "yo": "yo jhipster jdl bug-tracker.jh",
    "npx": "npx jhipster jdl bug-tracker.jh"

  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "generator-jhipster": "7.9.2"
  }
}

Unless these two commands are not equivalent?

bd82 commented 1 year ago

Actually these two commands may not be equivalent, I am not sure if yeoman does a local lookup in closest node_modules... but when I did npm install -g generator-jhipster before hand the scenario without npx still worked.

I am not sure if the arguments part is equivalent though...

mshima commented 1 year ago

At package.json scripts npx is not required, use plain jhipster jdl bug-tracker.jh

In this case, npx is just a wrapper for the local node_modules bin. So it’s just like

node_modules/.bin/jhipster jdl bug-tracker.jh

You can install globally. I’m just used to install locally.

The jhipster bin is required. Yo will not work.

bd82 commented 1 year ago

Okay I realize why 6months old version 10.1.2 suddenly broke in generator-jhipster

Basically chevrotain 10.1.2 was depending on @chevrotain/gast ^10.1.2 (with ^ minor range). So once chevrotain 10.2.0 was released installing chevrotain@10.1.2 would pull @chevrotain/gast@10.2.0 as a dependency, but between these two versions the TypeScript compilation target changed (es5 --> ES2015) so the inheritance system is incompatible between these two versions.

There are a few solutions:

  1. add a shrinkwrap file to generator-jhipster
  2. release 10.2.1 version of chevrotain with the es5 compilation target and then release version 11.0.0 with the new compilation target.

I will try to implement #2 but if you can't wait you may want to implement the first one.

I still don't know why you encountered issues with 10.2.0...

mshima commented 1 year ago

I did some debug and the update to 10.2.0 broke because of this change: https://github.com/Chevrotain/chevrotain/commit/a4caf4db21fc334d1733bdc946ff656f3d94bdc0#diff-3130c8a9d16f6271d9c85f52d411bf95164c311adf5c8c3283c0a716a4effb3c

Operators for...in and getOwnPropertyNames are quite different, so some methods were not checked before: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties#traversing_object_properties

Removing validateVisitor here makes the problem go await: https://github.com/jhipster/generator-jhipster/blob/045e1841226e237aa62aeae631b0a8a559266fca/jdl/parsing/validator.js#L336

But I have not clue if that's the correct approach.

bd82 commented 1 year ago

It is safe to remove the validateVisitor it is an optional flow.

I am evaluating removing it as it seems fragile and I suspect it should be something an end user implements according to their needs and specific flavor of class syntax (ES5/ES2015/mixings/other)

bd82 commented 1 year ago

I've released version 10.3.0 with ES5 artifacts and locked semver versions.

bd82 commented 1 year ago

The problem was resolved for me on the local package.json + npx scenario (after rm -rf node_modules && rm package-lock).

I still get an error with running directly with global npx package install, but I am guessing that is because chevrotain does not get reinstalled on every npx generator-jhipster@latest jdl bug-tracker.jh invocation (is there a clean flag?).

perhaps it would be a good safe practice to also release a new patch version of jhipster...

mshima commented 1 year ago

Transitional dependencies merged at https://github.com/jhipster/generator-jhipster/pull/19483. And 10.3.0 merged at https://github.com/jhipster/generator-jhipster/pull/19490.

Thanks @bd82

bd82 commented 1 year ago

hip hip hurray 😸