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

chore!: update to Node.js 20 #842

Merged
merged 15 commits into from May 8, 2023
Merged

chore!: update to Node.js 20 #842

merged 15 commits into from May 8, 2023

Conversation

rtritto
Copy link
Contributor

@rtritto rtritto commented May 1, 2023

Changes:

  • updated pnpm version from 7.9.5 to 8.4.0
  • added Node.js v20 (release date: 18 Apr 2023)
  • removed Node.js v14 (EOL: 30 Apr 2023)
  • in package.json of vite-plugin-ssr
    • locked minor dependency versions
    • updated @types/node to v20

@brillout
Copy link
Member

brillout commented May 1, 2023

Does it bring an added value?

@rtritto
Copy link
Contributor Author

rtritto commented May 1, 2023

Does it bring an added value?

This is a draft.

I did some changes, can you review and continue?

@brillout
Copy link
Member

brillout commented May 2, 2023

👍

Seems like pnpm has a bug with Node.js 20?

@rtritto
Copy link
Contributor Author

rtritto commented May 2, 2023

Yes, it's fixed in v8.3.1 (source: pnpm/pnpm#6424 (comment))

@rtritto
Copy link
Contributor Author

rtritto commented May 2, 2023

Seems that pnpm v8 only removes Node.js v14 (source: https://github.com/pnpm/pnpm/releases/tag/v8.0.0).
Lockfile needs to be regenerated.
Should we use this PR?

@brillout
Copy link
Member

brillout commented May 2, 2023

Should we use this PR?

Ok 👍

@rtritto
Copy link
Contributor Author

rtritto commented May 2, 2023

Related error: cacjs/cac#143

@rtritto
Copy link
Contributor Author

rtritto commented May 2, 2023

Related error: cacjs/cac#143

Fixed with 099e56e

@rtritto
Copy link
Contributor Author

rtritto commented May 2, 2023

Current version of pnpm seems that install exact version instead of the minor one 👀

Fixed lockfiles:

Example:

pnpm v7.9.5

lockfileVersion: 5.4
  vite-plugin-ssr:
    specifiers:
      cac: ^6.0.0
    dependencies:
      cac: 6.7.12    // correct: latest of minor

pnpm v8.4.0

lockfileVersion: 6
  vite-plugin-ssr:
    dependencies:
      cac:
        specifier: ^6.0.0
        version: 6.0.0  // problem: this version should be 6.7.12

Related: pnpm/pnpm#6463

@brillout
Copy link
Member

brillout commented May 3, 2023

Seems like the workaround is to set resolution-mode=highest?

@rtritto
Copy link
Contributor Author

rtritto commented May 3, 2023

I added resolution-mode=highest in ~/.npmrc and ~/vite-plugin-ssr/.npmrc and I regenerated .
It doesn't work (no changes in lockfile).
e.g.:

    es-module-lexer:
      specifier: ^0.10.0
      version: 0.10.0    // wrong: latest of v0 is 0.10.5
    esbuild:
      specifier: ^0.17.0
      version: 0.17.0    // wrong: latest is 0.17.18
    fast-glob:
      specifier: ^3.0.0
      version: 3.0.0    // wrong: latest is 3.2.12

Maybe the problem is related to workspace.

@brillout
Copy link
Member

brillout commented May 3, 2023

I think you have to remove the lock file before: $ rm pnpm-lock.yaml && pnpm install.

@rtritto
Copy link
Contributor Author

rtritto commented May 3, 2023

I think you have to remove the lock file before: $ rm pnpm-lock.yaml && pnpm install.

I always removed it before install.

@brillout
Copy link
Member

brillout commented May 3, 2023

Hm. That's weird then. Seems like a pnpm bug or maybe a typo.

@brillout
Copy link
Member

brillout commented May 3, 2023

If you want we can close an re-open later once pnpm's discussion about its new resolution strategy settles down?

@brillout
Copy link
Member

brillout commented May 3, 2023

I made some minor changes: ffaa24e.

@rtritto
Copy link
Contributor Author

rtritto commented May 3, 2023

If you want we can close an re-open later once pnpm's discussion about its new resolution strategy settles down?

If nothing changes, I would leave opened because it's visible to further comments from community

@brillout
Copy link
Member

brillout commented May 3, 2023

Ok

@rtritto
Copy link
Contributor Author

rtritto commented May 4, 2023

Anyway I suggest to set manually the minor version copying from previous lockfile (same ^ or exact version)
By this way, we safe know what are minor versions where the project starts to work

@brillout
Copy link
Member

brillout commented May 4, 2023

👍 I think that should work yes.

Once the PR is green we can think about what semver/resolution strategy we want to choose.

@rtritto
Copy link
Contributor Author

rtritto commented May 4, 2023

Anyway I suggest to set manually the minor version copying from previous lockfile (same ^ or exact version)
By this way, we safe know what are minor versions where the project starts to work

Done in acad008

Please can you remove/replace checks of Node v14?

@brillout
Copy link
Member

brillout commented May 5, 2023

👍 I'd suggest we create win16 and replace win14 with it.

@rtritto
Copy link
Contributor Author

rtritto commented May 5, 2023

In acad008, with esbuil@^0.17.10, pnpm has installed latest patch version 0.17.18 instead of latest minor version 0.17.10. 👀

In 63ed71f I locked the esbuil version to 0.17.10 (we are sure that this version works).

If all dependencies are now same before this PR, why does CI / Examples Vue/Others - Ubuntu - Node.js 16 fail? Any idea?

@rtritto
Copy link
Contributor Author

rtritto commented May 5, 2023

If all dependencies are now same before this PR, why does CI / Examples Vue/Others - Ubuntu - Node.js 16 fail? Any idea?

Dependencies of telefunc are outdated:

/telefunc-v1/package.json (PASS)

    "typescript": "^5.0.4",
    "vite": "^4.2.2",

/telefunc/package.json (FAIL)

    "typescript": "^4.8.4",
    "vite": "^3.0.9"

In 63ed71f I locked the esbuil version to 0.17.10 (we are sure that this version works).

I think it's safe to install esbuild v0.17.18 (esbuild@^0.17.18) due to same errors of CIs.

@rtritto
Copy link
Contributor Author

rtritto commented May 5, 2023

@brillout
Copy link
Member

brillout commented May 6, 2023

Indeed, the only blocker left is TypeStrong/ts-node#1997.

@rtritto
Copy link
Contributor Author

rtritto commented May 6, 2023

In 63ed71f I locked the esbuil version to 0.17.10 (we are sure that this version works).

I think it's safe to install esbuild v0.17.18 (esbuild@^0.17.18) due to same errors of CIs.

Should we update esbuild to v0.17.18 (esbuild@^0.17.18)?

Edit:
I updated to v0.17.18.

@brillout
Copy link
Member

brillout commented May 8, 2023

I think that we should update @types/node to v20 in:

While the VPS repository uses Node.js 20 we can't and shouldn't assume the user to use Node.js 20.

Should we update esbuild to v0.17.18 (esbuild@^0.17.18)?

Since Vite is a peer dependency, I'd suggest we take over Vite's dependency semvers.

@brillout brillout marked this pull request as ready for review May 8, 2023 10:13
@brillout brillout merged commit 63f9d2f into vikejs:main May 8, 2023
16 checks passed
@brillout
Copy link
Member

brillout commented May 8, 2023

Thanks for the PR 💚.

@rtritto rtritto deleted the node-20 branch May 8, 2023 10:50
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