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

loaders: use pre-compiled loader-runner distributed with Next.js #3823

Merged
merged 3 commits into from
Feb 21, 2023

Conversation

wbinnssmith
Copy link
Member

The benefit of this depends on vercel/next.js#45962, but it remains compatible with apps including loader-runner.

This first attempts to require loader-runner from the app's installed version of Next.js, falling back to requiring the package directly. We should probably eventually remove this fallback once all compatible versions of Next.js include the precompiled version, as that has a more predictable version of the package.

Test Plan: Linked a local copy of Next.js including vercel/next.js#45962 to an app without loader-runner that uses loaders and verified loaders ran.

The benefit of this depends on vercel/next.js#45962, but it remains compatible with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed version of Next.js, falling back to requiring the package directly. We should probably eventually remove this fallback once all compatible versions of Next.js include the precompiled version, as that has a more predictable version of the package.

Test Plan: Linked a local copy of Next.js including vercel/next.js#45962 to an app without `loader-runner` that uses loaders and verified loaders ran.
@vercel
Copy link

vercel bot commented Feb 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-basic-web 🔄 Building (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-designsystem-docs 🔄 Building (Inspect) Feb 17, 2023 at 11:39PM (UTC)
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 11:39PM (UTC)
7 Ignored Deployments
Name Status Preview Comments Updated
examples-cra-web ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-svelte-web ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)
examples-vite-web ⬜️ Ignored (Inspect) Feb 17, 2023 at 11:39PM (UTC)

@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2023

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

wbinnssmith added a commit to vercel/next.js that referenced this pull request Feb 15, 2023
Depends on #45962 and vercel/turborepo#3823.

This adds an example app that uses webpack loaders with Turbopack. It configures and uses the `@svgr/webpack` loader to transform svg assets into JavaScript as React components.

Test Plan: `pnpm next-with-deps --turbo ./examples/with-turbopack-loaders`
@github-actions
Copy link
Contributor

Benchmark for 5868925

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 8720.99µs ± 165.86µs 8903.09µs ± 124.86µs +2.09%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.89ms ± 0.23ms 12.59ms ± 0.32ms -2.35%
bench_hmr_to_commit/Turbopack RSC/1000 modules 482.55ms ± 0.95ms 481.56ms ± 1.41ms -0.21%
bench_hmr_to_commit/Turbopack SSR/1000 modules 8644.94µs ± 102.15µs 8702.07µs ± 51.09µs +0.66%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8319.34µs ± 90.61µs 8224.14µs ± 50.73µs -1.14%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.10ms ± 0.22ms 10.73ms ± 0.26ms -3.37%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8330.33µs ± 83.22µs 8392.45µs ± 81.64µs +0.75%
bench_hydration/Turbopack RCC/1000 modules 3755.20ms ± 23.87ms 3726.89ms ± 19.56ms -0.75%
bench_hydration/Turbopack RSC/1000 modules 3328.74ms ± 11.49ms 3342.48ms ± 13.49ms +0.41%
bench_hydration/Turbopack SSR/1000 modules 3478.22ms ± 27.35ms 3450.30ms ± 17.54ms -0.80%
bench_startup/Turbopack CSR/1000 modules 2651.06ms ± 6.47ms 2645.33ms ± 16.03ms -0.22%
bench_startup/Turbopack RCC/1000 modules 2238.55ms ± 7.49ms 2236.00ms ± 6.47ms -0.11%
bench_startup/Turbopack RSC/1000 modules 2175.90ms ± 5.18ms 2172.26ms ± 6.72ms -0.17%
bench_startup/Turbopack SSR/1000 modules 2067.79ms ± 2.01ms 2060.07ms ± 2.07ms -0.37%

@github-actions
Copy link
Contributor

Benchmark for 8bf2bb9

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 9350.10µs ± 85.20µs 9099.84µs ± 81.88µs -2.68%
bench_hmr_to_commit/Turbopack RCC/1000 modules 12.78ms ± 0.13ms 13.12ms ± 0.04ms +2.63%
bench_hmr_to_commit/Turbopack RSC/1000 modules 492.99ms ± 2.41ms 497.90ms ± 1.48ms +0.99%
bench_hmr_to_commit/Turbopack SSR/1000 modules 9273.47µs ± 70.21µs 9325.11µs ± 75.06µs +0.56%
bench_hmr_to_eval/Turbopack CSR/1000 modules 8390.80µs ± 63.48µs 8463.87µs ± 80.93µs +0.87%
bench_hmr_to_eval/Turbopack RCC/1000 modules 11.02ms ± 0.21ms 10.57ms ± 0.29ms -4.09%
bench_hmr_to_eval/Turbopack SSR/1000 modules 8376.61µs ± 89.08µs 8534.82µs ± 120.52µs +1.89%
bench_hydration/Turbopack RCC/1000 modules 3726.26ms ± 23.68ms 3690.51ms ± 22.18ms -0.96%
bench_hydration/Turbopack RSC/1000 modules 3364.59ms ± 10.72ms 3325.97ms ± 8.62ms -1.15%
bench_hydration/Turbopack SSR/1000 modules 3373.49ms ± 11.02ms 3399.65ms ± 13.43ms +0.78%
bench_startup/Turbopack CSR/1000 modules 2632.34ms ± 10.55ms 2639.64ms ± 7.73ms +0.28%
bench_startup/Turbopack RCC/1000 modules 2251.14ms ± 6.16ms 2261.41ms ± 3.47ms +0.46%
bench_startup/Turbopack RSC/1000 modules 2174.73ms ± 9.08ms 2165.84ms ± 4.08ms -0.41%
bench_startup/Turbopack SSR/1000 modules 2039.51ms ± 2.35ms 2046.58ms ± 3.40ms +0.35%


#[turbo_tasks::function]
pub async fn get_external_next_compiled_package_mapping(
package_name: StringVc,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
package_name: StringVc,
package_name: &str,

try {
({ runLoaders } = require("@vercel/turbopack/loader-runner"));
} catch {
({ runLoaders } = __turbopack_external_require__("loader-runner"));
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be removed

@wbinnssmith wbinnssmith merged commit 8fcf5d1 into main Feb 21, 2023
@wbinnssmith wbinnssmith deleted the wbinnssmith/use-compiled-loader-runner branch February 21, 2023 20:34
wbinnssmith added a commit to vercel/next.js that referenced this pull request Feb 21, 2023
ijjk pushed a commit to vercel/next.js that referenced this pull request Feb 21, 2023
jridgewell pushed a commit to vercel/next.js that referenced this pull request Mar 10, 2023
…cel/turborepo#3823)

The benefit of this depends on
#45962, but it remains compatible
with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed
version of Next.js, falling back to requiring the package directly. We
should probably eventually remove this fallback once all compatible
versions of Next.js include the precompiled version, as that has a more
predictable version of the package.

Test Plan: Linked a local copy of Next.js including
#45962 to an app without
`loader-runner` that uses loaders and verified loaders ran.
sokra pushed a commit to vercel/next.js that referenced this pull request Mar 13, 2023
…cel/turborepo#3823)

The benefit of this depends on
#45962, but it remains compatible
with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed
version of Next.js, falling back to requiring the package directly. We
should probably eventually remove this fallback once all compatible
versions of Next.js include the precompiled version, as that has a more
predictable version of the package.

Test Plan: Linked a local copy of Next.js including
#45962 to an app without
`loader-runner` that uses loaders and verified loaders ran.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 25, 2024
…cel/turborepo#3823)

The benefit of this depends on
#45962, but it remains compatible
with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed
version of Next.js, falling back to requiring the package directly. We
should probably eventually remove this fallback once all compatible
versions of Next.js include the precompiled version, as that has a more
predictable version of the package.

Test Plan: Linked a local copy of Next.js including
#45962 to an app without
`loader-runner` that uses loaders and verified loaders ran.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Jul 29, 2024
…cel/turborepo#3823)

The benefit of this depends on
#45962, but it remains compatible
with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed
version of Next.js, falling back to requiring the package directly. We
should probably eventually remove this fallback once all compatible
versions of Next.js include the precompiled version, as that has a more
predictable version of the package.

Test Plan: Linked a local copy of Next.js including
#45962 to an app without
`loader-runner` that uses loaders and verified loaders ran.
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 1, 2024
…cel/turborepo#3823)

The benefit of this depends on
#45962, but it remains compatible
with apps including `loader-runner`.

This first attempts to require `loader-runner` from the app's installed
version of Next.js, falling back to requiring the package directly. We
should probably eventually remove this fallback once all compatible
versions of Next.js include the precompiled version, as that has a more
predictable version of the package.

Test Plan: Linked a local copy of Next.js including
#45962 to an app without
`loader-runner` that uses loaders and verified loaders ran.
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