Skip to content

Conversation

Battjmo
Copy link

@Battjmo Battjmo commented Sep 16, 2025

Description

#4137

Bump isolated-vm to version 6.x.x, which supports Node 24: https://github.com/laverdet/isolated-vm?tab=readme-ov-file. I took a look through the commits on that release and didn't see anything that looked like it would impact functionality. The tests in the React package seem to all be borked on main so I can't confirm, took a stab at updating the contributing docs in there too to use yarn instead of npm.

I only did it in the React package since I don't want to get too far into the weeds and that's the one I use on my project.

Copy link

changeset-bot bot commented Sep 16, 2025

🦋 Changeset detected

Latest commit: 4356787

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@builder.io/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@samijaber
Copy link
Contributor

@Battjmo does the install work on your machine? It seems to now break in CI for v6.x

If yes, what node version are you using?

@Battjmo
Copy link
Author

Battjmo commented Sep 18, 2025

@Battjmo does the install work on your machine? It seems to now break in CI for v6.x

If yes, what node version are you using?

Yep, build worked in the react package, Node v24.4.1 or v22.12.0

v6 of what? I'm getting version 8.2.6 in the build output for the SDK version.

@samijaber
Copy link
Contributor

@Battjmo does the install work on your machine? It seems to now break in CI for v6.x
If yes, what node version are you using?

Yep, build worked in the react package, Node v24.4.1 or v22.12.0

v6 of what? I'm getting version 8.2.6 in the build output for the SDK version.

bumping isolated-vm to v6.0.0 fails in our CI, see https://github.com/BuilderIO/builder/actions/runs/17800387929/job/50623875294?pr=4145#step:4:584

I think it's because our CI is using node v18

- name: Use Node.js
uses: actions/setup-node@v4
with:
node-version: '18.x'

whereas isolated-vm v6 expects node >=22 https://github.com/laverdet/isolated-vm/blob/5dc622865679323a6b68dca7762e49d82d7f67ff/package.json#L7-L9

so i think this PR needs to also bump the node version across the board (all github .yml workflows, and .nvmrc)

v18.20.6

@Battjmo
Copy link
Author

Battjmo commented Sep 23, 2025

@Battjmo does the install work on your machine? It seems to now break in CI for v6.x
If yes, what node version are you using?

Yep, build worked in the react package, Node v24.4.1 or v22.12.0
v6 of what? I'm getting version 8.2.6 in the build output for the SDK version.

bumping isolated-vm to v6.0.0 fails in our CI, see https://github.com/BuilderIO/builder/actions/runs/17800387929/job/50623875294?pr=4145#step:4:584

I think it's because our CI is using node v18

- name: Use Node.js
uses: actions/setup-node@v4
with:
node-version: '18.x'

whereas isolated-vm v6 expects node >=22 https://github.com/laverdet/isolated-vm/blob/5dc622865679323a6b68dca7762e49d82d7f67ff/package.json#L7-L9

so i think this PR needs to also bump the node version across the board (all github .yml workflows, and .nvmrc)

v18.20.6

Makes sense, about time we get this thing off of 18 anyway. I'll get cracking on it.

@sidmohanty11
Copy link
Contributor

thanks for the PR @Battjmo! 🚀

@Battjmo
Copy link
Author

Battjmo commented Sep 24, 2025

Alright, I went ahead and disregarded my previous statement about not updating everything and just updated everything. Come ooooooon ci.

Copy link

nx-cloud bot commented Sep 24, 2025

View your CI Pipeline Execution ↗ for commit 4356787

Command Status Duration Result
nx test @snippet/hydrogen ✅ Succeeded 2m 51s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-26 14:16:50 UTC

@Battjmo
Copy link
Author

Battjmo commented Sep 24, 2025

All of the tests in /sdks pass for me locally. Pushed up this fix that I think might help, can't confirm since it works for me locally regardless.

@Battjmo
Copy link
Author

Battjmo commented Sep 25, 2025

Alright, I'm a little confused by the output here. It looks like I passed CI but maybe also not? @samijaber ?

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.

3 participants