Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update documentation to mention compatibility with ES2022 #2888

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

ekoleda-codaio
Copy link
Contributor

Update the overview page to mention support for ES2022 language features, now that the Packs runtime has been updated to run on Node 18.

I tried updating compile.ts to target ES2022 directly, but ran into some errors. This is fine though, as esbuild will just downlevel any newer features.

@ekoleda-codaio ekoleda-codaio added the documentation Related to the documentation in /docs. label Dec 28, 2023
@ekoleda-codaio ekoleda-codaio requested a review from a team as a code owner December 28, 2023 21:22
@huayang-codaio
Copy link
Contributor

I tried updating compile.ts to target ES2022 directly, but ran into some errors.

What error did you receive? Just curious. esbuild does mention that they support es2022.

@ekoleda-codaio
Copy link
Contributor Author

What error did you receive? Just curious. esbuild does mention that they support es2022.

 ./node_modules/.bin/ts-node cli/coda.ts execute documentation/samples/packs/hello_world/hello_world.ts Hello "Eric" --vm=false                                      [16:16:58]
/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/insert-module-globals/index.js:114
            var e = new SyntaxError(
                    ^
SyntaxError: Unexpected token (511:13) while parsing /var/folders/kp/k5sf9p811dg_yxm73vqnd_sr0000gq/T/coda-packs-7530a212-3ad4-4067-b694-3cba08499be4yHNhln/esbuild-bundle.js while parsing file: /var/folders/kp/k5sf9p811dg_yxm73vqnd_sr0000gq/T/coda-packs-7530a212-3ad4-4067-b694-3cba08499be4yHNhln/esbuild-bundle.js
    at DestroyableTransform.end [as _flush] (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/insert-module-globals/index.js:114:21)
    at DestroyableTransform.prefinish (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_transform.js:138:10)
    at DestroyableTransform.emit (node:events:513:28)
    at DestroyableTransform.emit (node:domain:489:12)
    at prefinish (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_writable.js:619:14)
    at finishMaybe (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_writable.js:627:5)
    at endWritable (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_writable.js:638:3)
    at DestroyableTransform.Writable.end (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_writable.js:594:41)
    at DestroyableTransform.onend (/Users/EricKoleda/code/packs-sdk/node_modules/.pnpm/[email protected]/node_modules/readable-stream/lib/_stream_readable.js:577:10)
    at Object.onceWrapper (node:events:627:28)

@huayang-codaio
Copy link
Contributor

Thanks. I am also taking a look.

@huayang-codaio
Copy link
Contributor

Actually. I am not exactly sure if the es2022 bundle will run in isolated-vm. We are running into this error because browserify doesn't accept some es2022 syntax.

Take a step back, we don't really need the target to be es2022 right? The goal is probably to allow monaco understand es2022 syntax (in the browser editor) but the bundle target (which is the binary that runs in isolated-vm) doesn't need to be es2022.

@ekoleda-codaio
Copy link
Contributor Author

From what I can tell, browserify uses the package insert-module-globals which in turn uses an old version of acorn to parse the code, and that old version doesn't support newer ES2022 syntax. Issue here: browserify/insert-module-globals#88

@huayang-codaio
Copy link
Contributor

I would suggest to stay on es2020 for the target for now. Let me take a quick look at monaco editor side.

From what I can tell, browserify uses the package insert-module-globals which in turn uses an old version of acorn to parse the code, and that old version doesn't support newer ES2022 syntax. Issue here: browserify/insert-module-globals#88

Right. Browserify is one issue. I am more worried if isolated-vm supports the full ES2022 syntax.

@ekoleda-codaio
Copy link
Contributor Author

Take a step back, we don't really need the target to be es2022 right? The goal is probably to allow monaco understand es2022 syntax (in the browser editor) but the bundle target (which is the binary that runs in isolated-vm) doesn't need to be es2022.

Yes, that is my understanding as well. Targeting ES2020 will I believe create shims for newer language features, which in theory could be a bit slower than using the native feature, but likely not enough to matter in practice.

@ekoleda-codaio ekoleda-codaio merged commit 3d104c3 into main Dec 28, 2023
1 check passed
@ekoleda-codaio ekoleda-codaio deleted the ek-es2022 branch December 28, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to the documentation in /docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants