Skip to content

feat(turbopack): externalType support global #80542

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

fireairforce
Copy link
Contributor

This pr introduces a new external type for turbopack(maybe not for next.js), it can help to generate external like: https://webpack.js.org/configuration/externals/#externalstypeglobal.

  • Support a new externalType global
  • Update the codengen content for external_module.rs

@ijjk ijjk added the Turbopack Related to Turbopack with Next.js. label Jun 15, 2025
@ijjk
Copy link
Member

ijjk commented Jun 15, 2025

Allow CI Workflow Run

  • approve CI run for commit: ca14499

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

CachedExternalType::Global => {
if self.request.is_empty() {
writeln!(code, "const mod = {{}};")?;
} else if self.request.contains('/') {
Copy link
Member

Choose a reason for hiding this comment

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

(asking because I did a quick search and couldn't easily find an answer) How does webpack do this? Does it use '/' or '.', or does it just not support nested objects in this string?

We could just make you import/require the top-level object, and then it's up to the import to do the property access. This is an external, so concerns like fine-grained dependency tracking and tree-shaking don't apply.

Copy link
Contributor Author

@fireairforce fireairforce Jun 17, 2025

Choose a reason for hiding this comment

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

this seems invalid here, webpack can deal the case like:

externals: {
  'antd/es/button': ['global', 'antd', 'es', 'button']
}

it can generate global['antd']['es']['button'], this is becase the request of webpack can accept both string and string[]: https://github.com/webpack/webpack/blob/main/lib/ExternalModule.js#L727

For turbopack case, the request now only accept a RcStr:

https://github.com/vercel/next.js/blob/canary/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs#L61

I think i can remove my branch here, and later i will send a pr to turbopack for the external.request to accept both RcStr and Vec<RcStr>, it will align with webpack

@@ -109,4 +111,5 @@ pub const TUBROPACK_RUNTIME_FUNCTION_SHORTCUTS: [(&str, &TurbopackRuntimeFunctio
("__turbopack_refresh__", TURBOPACK_REFRESH),
("__turbopack_require_stub__", TURBOPACK_REQUIRE_STUB),
("__turbopack_require_real__", TURBOPACK_REQUIRE_REAL),
("__turbopack_global_access__", TURBOPACK_GLOBAL_ACCESS),
Copy link
Member

Choose a reason for hiding this comment

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

Do you need to implement this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems the function has been removed, from pr: https://github.com/vercel/next.js/pull/78889/files

i will use globalThis directly instead of using __turbopack_context__.g here, they are equal

Copy link
Member

@bgw bgw left a comment

Choose a reason for hiding this comment

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

This has CI failures:

error[E0432]: unresolved import `crate::runtime_functions::TUBROPACK_RUNTIME_FUNCTION_SHORTCUTS`
Error:    --> turbopack/crates/turbopack-ecmascript/src/references/mod.rs:160:9
    |
160 |         TUBROPACK_RUNTIME_FUNCTION_SHORTCUTS, TURBOPACK_EXPORT_NAMESPACE, TURBOPACK_EXPORT_VALUE,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |         |
    |         no `TUBROPACK_RUNTIME_FUNCTION_SHORTCUTS` in `runtime_functions`
    |         help: a similar name exists in the module: `TURBOPACK_RUNTIME_FUNCTION_SHORTCUTS`

@fireairforce
Copy link
Contributor Author

TUBROPACK_RUNTIME_FUNCTION_SHORTCUTS

This is a typo from a merged pr, i will update 😂

@fireairforce fireairforce force-pushed the support-more-externals-type branch from e88474c to 3679dff Compare June 17, 2025 05:32
@fireairforce fireairforce force-pushed the support-more-externals-type branch from 3679dff to 0c0747f Compare June 17, 2025 05:40
Copy link

codspeed-hq bot commented Jun 17, 2025

CodSpeed Performance Report

Merging #80542 will not alter performance

Comparing fireairforce:support-more-externals-type (7be3ce8) with canary (e1b9236)

Summary

✅ 3 untouched benchmarks
⁉️ 9 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
⁉️ build[date-fns-all] 2.8 s N/A N/A
⁉️ build[date-fns-single] 1.6 s N/A N/A
⁉️ build[framer-motion-all] 3.9 s N/A N/A
⁉️ build[framer-motion-single] 2.7 s N/A N/A
⁉️ build[joy] 2.5 s N/A N/A
⁉️ build[lucide-react-all] 12.5 s N/A N/A
⁉️ build[lucide-react-single] 1 s N/A N/A
⁉️ build[mui] 3.9 s N/A N/A
⁉️ build[shiki] 6.3 s N/A N/A

@fireairforce
Copy link
Contributor Author

updated~

)?;
}
}
_ => {
Copy link
Member

@bgw bgw Jun 17, 2025

Choose a reason for hiding this comment

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

Nit: At this point, it's a bit better for this match to be exhaustive. If somebody adds a new enum variant, it'd be good for the compiler to warn them here and require they explicitly handle it.

Suggested change
_ => {
CachedExternalType::CommonJs | CachedExternalType::EcmaScriptViaRequire => {

Update: I pushed a commit to your PR with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx~

@ijjk
Copy link
Member

ijjk commented Jun 17, 2025

Failing test suites

Commit: 7be3ce8

__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/development/app-hmr/hmr.test.ts (PPR)

  • app-dir-hmr > filesystem changes > should update server components pages when env files is changed (node-module-var)
Expand output

● app-dir-hmr › filesystem changes › should update server components pages when env files is changed (node-module-var)

expect(received).toBe(expected) // Object.is equality

Expected: "mac"
Received: "ipad"

  145 |         // ensure it's restored back to "mac" before the next test
  146 |         await retry(async () => {
> 147 |           expect(await browser.elementByCss('p').text()).toBe('mac')
      |                                                          ^
  148 |         })
  149 |
  150 |         expect(next.cliOutput).not.toContain('FATAL')

  at toBe (development/app-hmr/hmr.test.ts:147:58)
  at retry (lib/next-test-utils.ts:811:14)
  at development/app-hmr/hmr.test.ts:146:9

Read more about building and testing Next.js in contributing.md.

pnpm test-dev-turbo test/e2e/app-dir/actions/app-action-node-middleware.test.ts (turbopack)

  • app-dir action handling > "use server" export values > should error when exporting non async functions at build time
Expand output

● app-dir action handling › "use server" export values › should error when exporting non async functions at build time

Expected Redbox but found no visible one.

  1002 |           )
  1003 |
> 1004 |           await assertHasRedbox(browser)
       |           ^
  1005 |           expect(await getRedboxSource(browser)).toContain(
  1006 |             'Only async functions are allowed to be exported in a "use server" file.'
  1007 |           )

  at Object.<anonymous> (e2e/app-dir/actions/app-action.test.ts:1004:11)

Read more about building and testing Next.js in contributing.md.

@bgw bgw enabled auto-merge (squash) June 18, 2025 06:08
@bgw bgw merged commit 57c304d into vercel:canary Jun 18, 2025
344 of 353 checks passed
@github-actions github-actions bot added the locked label Jul 2, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked Turbopack Related to Turbopack with Next.js.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants