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.

Sign up for GitHub

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

Jump to bottom

chore(deps): Update tree-sitter dependencies to versions supporting Node 20 #235

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

chore(deps): Update tree-sitter dependencies to versions supporting Node 20 #235

wants to merge 9 commits into from
+215 −755

Conversation

cr7pt0gr4ph7
Copy link

@cr7pt0gr4ph7 cr7pt0gr4ph7 commented Dec 30, 2024 •
edited
Loading

This PR upates the different tree-sitter dependencies to versions that are compatible with Node 19/20.

Background

The previously used version 0.20.x of tree-sitter (the exact version number depends on the specific tree-sitter-* sub-package) does not work on Node 19 and higher, leading to errors like the following:

Request initialize failed with message: bad export type for `tree_sitter_(tsx|csharp|typescript|...)_external_scanner_create`: undefined (Internal Error)

This broke e.g. the Official Cucumber VS Code extension when VS Code updated to a newer version of Node 19 (and later Node 20) that was not supported at the time by the tree-sitter library, but support has since been implemented and subsequently shipped in tree-sitter v0.21.0 and above.

Should fix the bad export type for ... error once the updated version is included cucumber/language-server and then cucumber/vscode that has been reported at least a couple times:

Sidenote on versioning & compatibility of tree-sitter

The versioning and release scheme of the tree-sitter libraries is a bit "messy": The tree-sitter-typescript/tree-sitter-rust/... sub-libraries have a strong dependency on the major version number of tree-sitter, such that versions of the component libraries that require different major versions of the tree-sitter library cannot be used together. But they are:

(though that does not strictly violate SemVer requirements due to still being a 0.x.y development version).

The approach taken here was to look for the newest version of tree-sitter for which all sub-libraries have at least one compatible release (which came out to be [email protected]) and to choose the newest version of each sub-library that still supports [email protected] (which eliminated a few of the most recent releases).

Related PRs

This PR subsumes/replaces #225 as well as the following automated PRs (which have build failures due to trying to update the different dependencies separately):

Tests

Tested with a custom build of cucumber/vscode to work on my current VS Code version:

Version: 1.95.3
Commit: f1a4fb101478ce6ec82fe9627c43efbf9e98c813
Date: 2024-11-13T14:50:04.152Z
Electron: 32.2.1
ElectronBuildId: 10427718
Chromium: 128.0.6613.186
Node.js: 20.18.0
V8: 12.8.374.38-electron.0
OS: Linux x64 6.11.3-300.fc41.x86_64

A note on "missing import symbols" errors

If there are issues with missing import symbols (namely missing symbol 'abort' from module 'env') after this update, this is due to a (too old) version of emscripten being used to compile tree-sitter to WASM.

The solution in that case is to install a newer version of emscripten.

  • tree-sitter currently uses emscripten v3.1.64
  • My development environment used emscripten v3.1.74

@cr7pt0gr4ph7
Copy link
Author

@kieran-ryan Test failures should be fixed now (npm test completed successfully when I ran it on my machine just now).

@kieran-ryan
Copy link
Member

@kieran-ryan Test failures should be fixed now (npm test completed successfully when I ran it on my machine just now).

Thanks a bunch for the contribution first and foremost @cr7pt0gr4ph7 - and for a well-documented pull request; will be super to have this issue resolved! Will look to fully test and review soon as I can - as time allows off on the back of holidays. Checking, are you testing on Windows by any chance; or any other other detail you can provide on your runtime? Will try to replicate difference in test outcome between running locally and the pipeline. There are some disparities in testing with the project which require resolution at some point to improve the contributing experience.

Suspect failing due to instability of query specifiers in tree-sitter language implementations (at least very early versions pinned in the package) which may have changed since with these updates - see tree-sitter-rust #226 for reference of associated changes. May not be the case, though was anticipating query changes with these upgrades.

Tree-sitter's live playground can be used to check the query and test data associated with a language-implementation for compatibility - compared to the older versions our queries were compatible with; though you may want to run a local tree-sitter playground against the specific language implementation versions you are running locally for accuracy as they may differ.

@cr7pt0gr4ph7
Copy link
Author

cr7pt0gr4ph7 commented Dec 31, 2024 •
edited
Loading

Edit: @kieran-ryan Problem solved - I commited the query fixes but didn't push them. Oops 😅

Checking, are you testing on Windows by any chance; or any other other detail you can provide on your runtime?

I'm running on Linux Bluefin (Fedora-based) inside a Docker container:

$ uname --all
Linux a8f8f4bfa59d 6.11.3-300.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Oct 10 19:18:36 UTC 2024 x86_64 GNU/Linux

$ node --version
v22.12.0

Will try to replicate difference in test outcome between running locally and the pipeline. There are some disparities in testing with the project which require resolution at some point to improve the contributing experience.

I'll have a look at the failed tests. (Edit: Solved, see above).

Suspect failing due to instability of query specifiers in tree-sitter language implementations (at least very early versions pinned in the package) which may have changed since with these updates - see tree-sitter-rust #226 for reference of associated changes. May not be the case, though was anticipating query changes with these upgrades.

The test failures which I could observe locally were partially due to syntax tree changes in tree-sitter-csharp. I'll have a look! (Edit: Already fixed, see above).

Tree-sitter's live playground can be used to check the query and test data associated with a language-implementation for compatibility - compared to the older versions our queries were compatible with; though you may want to run a local tree-sitter playground against the specific language implementation versions you are running locally for accuracy as they may differ.

Thanks for the pointer! I actually looked for a Tree-sitter playground, but Google did not provide any helpful results. The manual page you linked does not even appear on the first page of results...

@cr7pt0gr4ph7
Copy link
Author

@kieran-ryan Regarding further testing: I'd probably suggest updating cucumber/vscode first, and then wait for some time to if any additional problems are reported. It can't get any more broken than it is right now (where the language server doesn't even start in VS Code).

kieran-ryan
kieran-ryan requested changes Jan 2, 2025
View reviewed changes
Copy link
Member

@kieran-ryan kieran-ryan left a comment

Choose a reason for hiding this comment

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

@cr7pt0gr4ph7, would you be aware of the minimum supported version of VSCode following these changes; along with the range of supported node versions?

.github/workflows/test-javascript.yml Outdated Show resolved Hide resolved
.github/workflows/release-npm.yml Outdated Show resolved Hide resolved
@kieran-ryan
Copy link
Member

@cr7pt0gr4ph7, can for sure understand that perspective; would just be conscious of current users using the language server directly or through the VSCode extension that have pinned an earlier version of VSCode - as would autoupdate and could require promptly jumping on any fixes.

@cr7pt0gr4ph7
Copy link
Author

cr7pt0gr4ph7 commented Jan 2, 2025 •
edited
Loading

Would you be aware of the minimum supported version of VSCode following these changes; along with the range of supported node versions?

I haven't modified anything related to communication with VS Code or upgraded any dependency beside tree-sitter and it's sub-libraries.

It therefore all comes down to the compatibility between tree-siter and emscripten version we use to compile to ASM on one side, and the Node.js versions included in different VS Code versions on the other.

  • There isn't an offical table of the Node.js versions shipped for each VS Code release, but utilizing the script from Sneezry/vscode-version-watcher (with some tweaks to make it run again) produces the table below.
  • Both emscripten v3.1.64 and tree-sitter unfortunately do not seem to have an official matrix of supported Node versions; running our tests in the different versions of Node should be enough to verify compatibility, though. I updated the GitHub workflow file accordingly. Would you be so kind to trigger a run?

Last update: 2025-01-02 21:02:51 GMT

VS Code Electron Node Chrome
Latest 32.2.7 20.18.1 128.0.6613.186
1.96.2 32.2.6 20.18.1 128.0.6613.186
1.96.1 32.2.6 20.18.1 128.0.6613.186
1.96.0 32.2.6 20.18.1 128.0.6613.186
1.95.3 32.2.1 20.18.0 128.0.6613.186
1.95.2 32.2.1 20.18.0 128.0.6613.186
1.95.1 32.2.1 20.18.0 128.0.6613.186
1.95.0 32.2.1 20.18.0 128.0.6613.186
1.94.2 30.5.1 20.16.0 124.0.6367.243
1.94.1 30.5.1 20.16.0 124.0.6367.243
1.94.0 30.5.1 20.16.0 124.0.6367.243
1.93.1 30.4.0 20.15.1 124.0.6367.243
1.93.0 30.4.0 20.15.1 124.0.6367.243
1.92.2 30.1.2 20.14.0 124.0.6367.243
1.92.1 30.1.2 20.14.0 124.0.6367.243
1.92.0 30.1.2 20.14.0 124.0.6367.243
1.91.1 29.4.0 20.9.0 122.0.6261.156
1.91.0 29.4.0 20.9.0 122.0.6261.156
1.90.2 29.4.0 20.9.0 122.0.6261.156
1.90.1 29.4.0 20.9.0 122.0.6261.156
1.90.0 29.4.0 20.9.0 122.0.6261.156
1.89.1 28.2.8 18.18.2 120.0.6099.291
1.89.0 28.2.8 18.18.2 120.0.6099.291
1.88.1 28.2.8 18.18.2 120.0.6099.291
1.88.0 28.2.8 18.18.2 120.0.6099.291

| 1.87.2 | 27.3.2 | 18.17.1 | 118.0.5993.159 |
| 1.87.1 | 27.3.2 | 18.17.1 | 118.0.5993.159 |
| 1.87.0 | 27.3.2 | 18.17.1 | 118.0.5993.159 |
| 1.86.2 | 27.2.3 | 18.17.1 | 118.0.5993.159 |
| 1.86.1 | 27.2.3 | 18.17.1 | 118.0.5993.159 |
| 1.86.0 | 27.2.3 | 18.17.1 | 118.0.5993.159 |
| 1.85.2 | 25.9.7 | 18.15.0 | 114.0.5735.289 |
| 1.85.1 | 25.9.7 | 18.15.0 | 114.0.5735.289 |
| 1.85.0 | 25.9.7 | 18.15.0 | 114.0.5735.289 |
| 1.84.2 | 25.9.2 | 18.15.0 | 114.0.5735.289 |
| 1.84.1 | 25.9.2 | 18.15.0 | 114.0.5735.289 |
| 1.84.0 | 25.9.2 | 18.15.0 | 114.0.5735.289 |
| 1.83.1 | 25.8.4 | 18.15.0 | 114.0.5735.289 |
| 1.83.0 | 25.8.4 | 18.15.0 | 114.0.5735.289 |
| 1.82.3 | 25.8.1 | 18.15.0 | 114.0.5735.289 |
| 1.82.2 | 25.8.1 | 18.15.0 | 114.0.5735.289 |
| 1.82.1 | 25.8.0 | 18.15.0 | 114.0.5735.289 |
| 1.82.0 | 25.8.0 | 18.15.0 | 114.0.5735.289 |
| 1.81.1 | 22.3.18 | 16.17.1 | 108.0.5359.215 |
| 1.81.0 | 22.3.18 | 16.17.1 | 108.0.5359.215 |
| ... | ... | ... | ... |

(Table was truncated for readability to only show the history up until the last known-good version 1.89 the current minimum supported version 1.82.0 of VS Code)

Would just be conscious of current users using the language server directly or through the VSCode extension that have pinned an earlier version of VSCode - as would autoupdate and could require promptly jumping on any fixes.

You're absolutely right; I hadn't considered that scenario...

cr7pt0gr4ph7
cr7pt0gr4ph7 commented Jan 2, 2025
View reviewed changes
.github/workflows/release-npm.yml Outdated Show resolved Hide resolved
.github/workflows/test-javascript.yml Outdated Show resolved Hide resolved
@cr7pt0gr4ph7
Copy link
Author

cr7pt0gr4ph7 commented Jan 2, 2025 •
edited
Loading

I just had a look at the new CI test results, which seem good so far. A few notes:

  • The Windows tests always succeed because they don't even run the WASM tests... (because they don't install the tree-sitter CLI tool on Windows)

    > @cucumber/[email protected] prepare
> husky && node scripts/build.js && cp node_modules/web-tree-sitter/tree-sitter.wasm dist

Skipping compilation of tree-sitter wasms - node_modules\.bin\tree-sitter is not installed

  • The 19.x tests fail because @cucumber/cucumber and the @typescript-eslint/* packages are marked as incompatible with Node 19 (probably because it is a non-LTS release that is already past its six month support window). Though VS Code skipped straight from 19.x to 20.x, so it would probably be fine to restrict support to 18.x and 19.x.

So, in conclusion: It should likely be running just fine in VS Code 1.82.0, which uses Node 18.15.0. I'm unfortunately not able to verify this locally with an old version of VS Code due to the way my system is set up.

@cr7pt0gr4ph7
Copy link
Author

cr7pt0gr4ph7 commented Jan 3, 2025 •
edited
Loading

I updated test-javascript.yaml to run the tests on every current LTS Node release (18.x, 20.x, 22.x, 24.x) because while VS Code uses Node 20.x right now, other clients might use newer versions.

Based on the successful tests on Node 18.x and 20.x (and conditional on whether the tests for Node 22.x and 24.x also succeed) this should be safe to release, I think.

@kieran-ryan Thanks for the quick responses, very much appreciated!

Edit: Would it make sense to define the official support policy of cucumber/language-service as "all currently maintained LTS releases of Node as defined by https://nodejs.org/en/about/previous-releases"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kieran-ryan kieran-ryan kieran-ryan requested changes

Assignees
No one assigned
Labels
🤖 dependencies
Projects
None yet
Milestone
No milestone
2 participants
@cr7pt0gr4ph7 @kieran-ryan