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

Upgrade Node.js to fix repro templates (16 -> LTS) #57871

Closed
wants to merge 2 commits into from
Closed

Upgrade Node.js to fix repro templates (16 -> LTS) #57871

wants to merge 2 commits into from

Conversation

karlhorky
Copy link
Contributor

@karlhorky karlhorky commented Nov 1, 2023

What?

Upgrade Node.js version from 16 to LTS in CodeSandbox reproduction templates for both App Router and Pages Router

Why?

Currently the reproduction templates linked in the GitHub issues template is broken. This causes users reporting bugs to not be able to create reproductions.

(Next.js v14 has a minimum Node.js version of 18.17)

Reproduction steps:

  1. Open either reproduction template on CodeSandbox, eg. the Pages Router reproduction template: https://codesandbox.io/p/sandbox/github/vercel/next.js/tree/canary/examples/reproduction-template-pages
  2. Delete the outdated yarn.lock file to get the latest canary version (this I fixed separately in Remove outdated yarn.lock in CodeSandbox repros #57895 and Add empty yarn.lock to prevent caching #57896 )
  3. Open a new terminal and run yarn to install latest canary version
  4. The latest canary cannot be installed, because of the CodeSandbox Node.js version of 16.17.0
$ yarn
yarn install v1.22.19
info No lockfile found.
[1/4] Resolving packages...
[2/4] Fetching packages...
error next@14.0.2-canary.5: The engine "node" is incompatible with this module. Expected version ">=18.17.0". Got "16.17.0"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Screenshot 2023-11-01 at 15 31 31

How?

Change the base image to Node.js LTS bullseye image, using Dev Containers

CodeSandbox Demo of a fixed sandbox which applies this change: https://codesandbox.io/p/sandbox/zealous-ellis-t599tx?file=%2Fapp%2Fpage.tsx%3A4%2C2

Screenshot 2023-11-01 at 17 10 14

@karlhorky karlhorky requested review from a team as code owners November 1, 2023 10:56
@karlhorky karlhorky requested review from ismaelrumzan and molebox and removed request for a team November 1, 2023 10:56
@ijjk ijjk added the examples Issue/PR related to examples label Nov 1, 2023
@ijjk
Copy link
Member

ijjk commented Nov 1, 2023

Allow CI Workflow Run

  • approve CI run for commit: f2baaf9

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

1 similar comment
@ijjk
Copy link
Member

ijjk commented Nov 1, 2023

Allow CI Workflow Run

  • approve CI run for commit: f2baaf9

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@ijjk
Copy link
Member

ijjk commented Nov 1, 2023

Allow CI Workflow Run

  • approve CI run for commit: bd7d101

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

I think this should not be required, rather we should rely on the fact that CodeSandbox has this by default. (I pinged them about it).

Also, the reproduction template might not be used in CodeSandbox, so it doesn't feel correct to add this file.

@karlhorky
Copy link
Contributor Author

karlhorky commented Nov 1, 2023

we should rely on the fact that CodeSandbox has this by default

Agree that this would be ideal! However, this could take months or years though, judging by how slowly Replit updated Node.js. It may be in a critical part of infrastructure that is not easy to change.

In the meantime, the reproduction template is broken for users trying to report a bug, which is in my opinion very important to fix immediately.

(it may also be broken for the App Router reproduction template, I didn't check it yet)

Also, the reproduction template might not be used in CodeSandbox, so it doesn't feel correct to add this file.

If it would be preferable to have it in .devcontainers, apparently this works too:

Screenshot 2023-11-01 at 15 24 13

@karlhorky
Copy link
Contributor Author

@balazsorban44 I added reproduction steps and a screenshot to show the breakage, since that may have been unclear.

@karlhorky karlhorky changed the title Upgrade Node.js in pages repro template (16 -> LTS) Upgrade Node.js in repro templates (16 -> LTS) Nov 1, 2023
@karlhorky karlhorky changed the title Upgrade Node.js in repro templates (16 -> LTS) Upgrade Node.js to fix repro templates (16 -> LTS) Nov 1, 2023
@karlhorky
Copy link
Contributor Author

If it would be preferable to have it in .devcontainers, apparently this works too:

@balazsorban44 I switched this PR to Dev Containers, which are not CodeSandbox-specific (and which already exist in various places in the Next.js repo).

@karlhorky
Copy link
Contributor Author

I chatted a bit with @CompuIves, and my current solution with Dev Containers would also be a preferable solution for CodeSandbox.

@balazsorban44 the CodeSandbox reproductions are currently broken for users. How about going ahead with the Dev Containers version? (instead of waiting until some future point in time when CodeSandbox upgrades beyond v16)

Another benefit with the Dev Containers version: it allows for the Next.js reproduction examples to specify the best specific Node.js version to run them on.

@balazsorban44
Copy link
Member

Ideally, we should not need to add any additional files here. Awaiting response from our chat with the CodeSandbox team.

@balazsorban44
Copy link
Member

CodeSandbox now defaults to Node.js 20 on new sandboxes, so this is not necessary anymore!

@karlhorky
Copy link
Contributor Author

Ok, thanks.

In future this may come back again (eg. CodeSandbox is behind on Node.js versions and Next.js has already upgraded to a minimum beyond the CodeSandbox version), but I guess maintainers will revisit it then.

@karlhorky karlhorky deleted the patch-2 branch November 27, 2023 12:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue/PR related to examples locked
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants