NeilFraser / JS-Interpreter

A sandboxed JavaScript interpreter in JavaScript.
Apache License 2.0
1.96k stars 352 forks source link

js-interpreter including ESM and in the npm ecosystem #268

Open mercmobily opened 4 months ago

mercmobily commented 4 months ago

Alright, I did it. This is finally something I am actually proud of. So, here are the changes in a nutshell:

acorn.js

I changed the export code at the top so that it's a little neater, AND it's all ready for the possible tiny code injection at the top to turn this into an ESM. I got rid of AMD support (which would have made things way more difficult). I got rid of the globalThis pattern. It can be re-added, but... I am not 10000% sure what pattern that is; let me know if it's something that's really absolutely wanted.

(Please note that I am a big fan of keeping things simple and easy. I am sure there are plenty of magic tools out there that are able to turn the given file into a magic UMD. The tool you use depends on the current fashion; I've seen so many... Grunt, Gulp, Webpack, Rollup, Parcel... then the current fashion is Vite but then there is BUN... Sorry, I prefer 9 lines of code, especially now that we can FINALLY say that AMD is history. Hopefully, CJS and "browser globals" will also be history soon enough, and we will no longer need any building, etc. Till then, it's 9 lines of code...)

So, here is a summary of the changes:

interpreter.js

I added the header to make it multi-format (commonJS, browser's global, and ESM if a tiny injection at the top is added). It now uses a well established multi-format pattern. So, I got rid of the Interpreter.nativeGlobal variable which was based on globalThis etc. Another thing I did -- partially as a consequence of the nativeGlobal thing` was to change this:

    value = arguments.length ? Interpreter.nativeGlobal.String(value) : '';

Into this:

    value = arguments.length ? String(value) : '';

EDIT: No I didn't. I made this change, and then fixed it when I realised that the point is to use the original methods before redefining them. I did it but without using the now removed nativeGlobal -- see change to see**

The same for Boolean, Number, Date, etc. I am still not sure what globalThis is about. I looked at the issues, and there is something about problems with using import on it (which I am AMAZED it would ever even work). But it looks like it's something that is addresses with this PR.

createEsmFiles.js

This is a new file. It injects TINY amounts of code into interpreter.js and acorn.js so that they export the right variables. This building step is necessary. The added code is very minimal. This is added to interpreter.js:

import { parse, version } from './acorn-esm.js';
var asEsm = true;
export { Interpreter, parse, version as acornVersion };

And this is added to acorn.js:

export { parse, version };
var asEsm = true;

That's it! This will create interpreter-esm.js and acorn-esm.js as fully usable esm modules. Because the step is so tiny, I actually do this as a postinstall task in NPM.

This could potentially be a problem as people will only get those files after a npm install. If they clone it, they will need to run generateEsmFiles.js to generate them.

Alternatively, we can turn this into a build task, and remove the -esm files from .gitignore.

index-esm.html

This is the same as index.html, but it loads the interpreter as an ESM module instead. It works exactly the same.

We need to decide what to do about the acorn_interpreter.js file. I am sure we need to keep it, or existing users that expect it will have a heart attack. I THINK the building process should work the exact same (or maybe THAT is the piece of the puzzle that needs nativeGlobal?!?).

I need to test this in a proper NPM project, to check that tools like vite load everything fine etc. But I don't anticipate huge problems there.

I hope this helps.

mercmobily commented 3 months ago

I did some extra testing, discovered why it was important ti have Interpreter.nativeGlobal.Number() and not just Number()... My apologies, it should have been obvious. I did extra testing, and things seem to work now

mercmobily commented 3 months ago

Is this something you are considering?

Kreijstal commented 3 months ago

maybe you will be the new mantainer lol

mercmobily commented 3 months ago

I doubt it. I am only humbly suggesting a possible solution to repackage this amazing module so that it's usable on the server and as an ESM. I did nothing -- Neil's the gifted one around here...

On Wed, 10 Apr 2024 at 16:14, Kreijstal @.***> wrote:

maybe you will be the new mantainer lol

— Reply to this email directly, view it on GitHub https://github.com/NeilFraser/JS-Interpreter/pull/268#issuecomment-2047668806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQHWXQGHE25BIUXIDIHWD3Y4VCMJAVCNFSM6AAAAABFFRXVRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBXGY3DQOBQGY . You are receiving this because you authored the thread.Message ID: @.***>