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 Crafter, drop Node.js 12 support, move to Node.js 18 #15

Merged
merged 7 commits into from
Jun 9, 2023

Conversation

igoradamenko
Copy link
Contributor

No description provided.

@igoradamenko igoradamenko requested a review from Ge11ert June 8, 2023 11:55
@igoradamenko
Copy link
Contributor Author

@Ge11ert, eh, feels like one does not simply update Crafter, does they?

https://github.com/funbox/vscode-apib-ls/actions/runs/5210878143/jobs/9402497422

@Ge11ert
Copy link
Contributor

Ge11ert commented Jun 8, 2023

@Ge11ert, eh, feels like one does not simply update Crafter, does they?

https://github.com/funbox/vscode-apib-ls/actions/runs/5210878143/jobs/9402497422

No preliminary thoughts, to be honest. Will look into it

@@ -76,7 +76,7 @@
"scripts": {
"vscode:prepublish": "npm run clean-dist-dir && npm run esbuild-min",
"clean-dist-dir": "rm -rf dist/",
"esbuild-base": "esbuild ./client/extension.js ./server/server.js --bundle --outdir=dist --define:process.env.BASE_DIR='\"dist\"' --external:vscode --format=cjs --platform=node",
"esbuild-base": "esbuild ./client/extension.js ./server/server.js --bundle --outdir=dist --define:process.env.BASE_DIR='\"dist\"' --external:vscode --format=cjs --platform=node --target=es2018",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igoradamenko The Empire Crafter did nothing wrong.

I've tried to update some packages and run vscode-test/sample using different versions of Node, but no luck. Then I managed to find out the original crash error.

Here is the trick:

vscode-jsonrpc itself includes some relatively fresh ES features like optional chaining: https://github.com/microsoft/vscode-languageserver-node/blob/main/jsonrpc/src/common/linkedMap.ts#L59

So the error was

vscode-apib-ls/dist/server/server.js:320
        return this._head?.value;
                          ^
SyntaxError: Unexpected token '.'

Since we use relatively old version of VScode in our tests (1.48), I guess that Electron wrapper of that version contains slightly outdated chromium, which does not support optional chaining.

Now, esbuild doc says:

by default, esbuild's output will take advantage of all modern JS features. For example, a !== void 0 && a !== null ? a : b will become a ?? b when minifying is enabled which makes use of syntax from the ES2020 version of JavaScript.

Since I don't want to force users to use brand new VSCode versions, I set target ES version for esbuild to ES2018, so it can transpile optional chaining and other js features.

As you see, now tests are green =)

Copy link
Contributor Author

@igoradamenko igoradamenko Jun 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sweet taste of ES syntactic sugar! Yummm.

Thank you for the research and the fix 🤝

@igoradamenko igoradamenko merged commit f2b4de6 into master Jun 9, 2023
3 checks passed
@igoradamenko igoradamenko deleted the feature/node18 branch June 9, 2023 10:55
@igoradamenko
Copy link
Contributor Author

igoradamenko commented Jun 9, 2023

Sent the release archive into VSCode Marketplace:

image

Will be published within an hour, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants