-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
backport: next-swc setup #69238
base: 14-2-1
Are you sure you want to change the base?
backport: next-swc setup #69238
Conversation
Hi there 👋 It looks like this PR introduces broken links to the docs, please take a moment to fix them before merging:
Thank you 🙏 |
8d8af59
to
2e5050e
Compare
Failing test suitesCommit: ac3ffae
Expand output● middleware - development errors › when there is a compilation error after boot › logs the error correctly
Read more about building and testing Next.js in contributing.md. |
7ea77dd
to
460fec6
Compare
## What? In the past folks on the team have reported that anytime the SWC / Turbopack binary changes they had to suddenly run `pnpm build-native` in `packages/next-swc`, even though they didn't build from source before. While investigating optimizing packing for tests I found that it's copying the native binary into the original repository not the temporary repository, which then keeps the file there, even after the tests finish. This fixes the path to use the tmp repository instead. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: ## For Contributors ### Improving Documentation - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide ### Adding or Updating Examples - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md ### Fixing a bug - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ### Adding a feature - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md ## For Maintainers - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change ### What? ### Why? ### How? Closes NEXT- Fixes # --> Closes NEXT-3157
Before: 25.71s ![CleanShot 2024-04-23 at 12 19 42@2x](https://github.com/vercel/next.js/assets/6324199/3a0ebb81-ac55-4b0c-8bfc-9a61ce138e6f) After: 11.05s (-57%) ![CleanShot 2024-04-23 at 12 16 35@2x](https://github.com/vercel/next.js/assets/6324199/d7b6cd4c-d1e4-4dc2-a423-20b539186d25) Currently the system for isolation looks like this: - Copy `packages` folder to an isolated directory - Run `pnpm pack` for all folders in `packages` - Collect the pack files, add them as `dependencies` in package.json of the isolated application - Run `pnpm install` Because the `next-swc` (Turbopack + SWC, yes we still need to rename the package) binary file is quite large in development (900MB+) it means we have to copy, then zip (pnpm pack), then unzip (pnpm install) that binary, which takes about 3+ seconds in each step. The change in this PR is to skip the copy/zip/unzip completely by providing the folder path for the binary directly to Next.js, as it's a binary we don't need the special isolation for this, it's already standalone so running it directly allows us to skip 14,6 seconds of work that is required for each isolated test in development. This will likely have little effect on CI times as we already do some tricks like only running the packing at the start of the CI process. Only thing that could be better and is probably worth doing is adopting this change for the time it saves for unzipping, that's almost 4 seconds per test still. <!-- Thanks for opening a PR! Your contribution is much appreciated. To make sure your PR is handled as smoothly as possible we request that you follow the checklist sections below. Choose the right checklist for the change(s) that you're making: - Run `pnpm prettier-fix` to fix formatting issues before opening the PR. - Read the Docs Contribution Guide to ensure your contribution follows the docs guidelines: https://nextjs.org/docs/community/contribution-guide - The "examples guidelines" are followed from our contributing doc https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md - Make sure the linting passes by running `pnpm build && pnpm lint`. See https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md - Related issues linked using `fixes #number` - Tests added. See: https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md - Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. (A discussion must be opened, see https://github.com/vercel/next.js/discussions/new?category=ideas) - Related issues/discussions are linked using `fixes #number` - e2e tests added (https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) - Documentation added - Telemetry added. In case of a feature if it's used or not. - Errors have a helpful link attached, see https://github.com/vercel/next.js/blob/canary/contributing.md - Minimal description (aim for explaining to someone not on the team to understand the PR) - When linking to a Slack thread, you might want to share details of the conclusion - Link both the Linear (Fixes NEXT-xxx) and the GitHub issues - Add review comments if necessary to explain to the reviewer the logic behind a change Closes NEXT- Fixes # --> Closes NEXT-3200 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
Noticed while testing turbopack version of tests that we weren't loading the correct swc binary since this path wasn't absolute and was just the relative path in `node_modules`. --------- Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
### What this was the way we run ci on turbopack's repo and to get the latest test results, which isn't required anymore.
460fec6
to
fbc9a3f
Compare
fbc9a3f
to
ac3ffae
Compare
@@ -4,6 +4,8 @@ | |||
#![feature(arbitrary_self_types)] | |||
#![feature(iter_intersperse)] | |||
|
|||
// a fake next-swc change (again) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was just to trigger a rebuild of next-swc
, remove before merging
A couple conveniences for next-swc that are nice to have for e.g. testing the backport branch locally