PhoenicisOrg / scripts

Phoenicis scripts
GNU Lesser General Public License v3.0
64 stars 49 forks source link

Fix: include returns undefined for classes #1075

Closed plata closed 5 years ago

plata commented 5 years ago

For

class test { myMethod() {}}

it gives "undefined". For

class test { myMethod() {}}
test

it gives "class test { myMethod() {}}".

This is caused by a bugfix in graaljs.

Affected:

Possible solutions:

see https://github.com/PhoenicisOrg/phoenicis/pull/2040

madoar commented 5 years ago

I'm in favor of the third solution:

use old graaljs version (19.0.0)

because it is the quickest solution, leads to no problems for us as far as I know, and is also the easiest solution to reverse when we found a solution for https://github.com/PhoenicisOrg/phoenicis/issues/2042

plata commented 5 years ago

I would vote for one of the other solutions because

madoar commented 5 years ago

What about extending our own include mechanism to provide explicit exports? We could even try to execute included scripts in an isolated environment (i.e. their own polyglot context) and inject the populated result object in the calling script.

plata commented 5 years ago

What do we gain with explicit exports? I don't think an isolated environment would work e.g. for engine plugins.

madoar commented 5 years ago

Explicit exports make the code easier to understand. In addition we are able to export multiple things in one script.

About executing the included scripts in an isolated environment: It seems like this is blocked by https://github.com/oracle/graal/issues/631

madoar commented 5 years ago

@plata I think this is fixed by #1076. Can we close this?