Skip to content

Conversation

@petele
Copy link
Contributor

@petele petele commented Jul 16, 2024

Quickly select the right version of node when using nvm for the repo.

@ddbeck
Copy link
Collaborator

ddbeck commented Jul 16, 2024

Huh, does nvm support .tool-versions? I use asdf for this and have it ignored, but it would be nice to check in something like it.

(I'll see if asdf could use .nvmrc too.)

@petele
Copy link
Contributor Author

petele commented Jul 16, 2024

Interesting - I don't use asdf, and am not familiar with it. But, it doesn't look like it does. :(

.nvmrc Outdated
@@ -0,0 +1 @@
22
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, I couldn't find a way to consolidate on a single file. How about we just set this to be consistent with .node-version and be done with it?

Suggested change
22
18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried running it with 18.16.1 and get an error that I don't get with 22.4.1. @tropicadri mentioned she was having problems with 18 as well and that 22 worked for her. I'm OK with going with 18, but we'll need to specify a subversion as well.

Which version are you running?

file:///Users/petele/Sites/web-features/packages/compute-baseline/dist/browser-compat-data/compat.js:1
import bcd from "@mdn/browser-compat-data" with { type: "json" };
                                           ^^^^

SyntaxError: Unexpected token 'with'
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:119:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:468:14)

Node.js v18.16.1

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah ha. It needs to be >=18.20, which is when import attributes landed in Node.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it - changed it to the official Hydrogen build

@petele petele requested a review from ddbeck July 19, 2024 16:07
@ddbeck ddbeck merged commit 7792484 into web-platform-dx:main Jul 26, 2024
@petele petele deleted the npm-version branch July 26, 2024 14:01
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.

2 participants