mirror of
https://github.com/hanzoai/next.js.git
synced 2026-10-30 14:46:38 +00:00
Luke SandbergandGitHub
35b5582647
## What? Remove `#[turbo_tasks::function]` from all methods on the `Issue` value_trait, following the same pattern as #92593 which did the same for `ValueDebug`. The `Issue` trait previously had 8 methods decorated with `#[turbo_tasks::function]`, which caused the turbo-tasks machinery to generate `NativeFunction` statics, task registration, vtable entries, and scheduling/caching infrastructure for each method on each of the ~48 `impl Issue for` types in the codebase — most of which are trivially synchronous (returning a constant string, enum variant, or stored struct field). ## How? - Change all 8 `Issue` trait methods to plain methods. Methods that are always synchronous become direct return-type methods (`severity`, `stage`, `source`, `documentation_link`). Methods that are genuinely async on at least one impl use `Pin<Box<dyn Future<Output = Result<T>> + Send + '_>>` for object safety. - Callers that need to invoke trait methods on a `Vc<Box<dyn Issue>>` use `issue.into_trait_ref().await?` to get a `TraitRef<Box<dyn Issue>>` and call methods directly on that. - `PlainIssue::from_issue` (the sole hot caller that materializes all fields) resolves one `trait_ref` and calls all methods on it, similar to the `ValueDebug` pattern. - `ResolvingIssueWithLocation` (which wraps an inner `ResolvedVc<Box<dyn Issue>>`) eagerly copies `severity`, `stage`, `documentation_link`, and `source` at construction time rather than delegating async calls to the inner issue. - Delete `OptionIssueSource` and `OptionStyledString` wrapper types, which existed solely as `Vc<T>` return type wrappers for the old turbo-tasks methods. ## Tradeoffs **Lost per-call caching:** Previously each `Issue` method was a turbo-tasks task, so results were cached per `(Vc<Issue>, method)`. In practice this caching was useless — issues are emitted once and their fields read once by `PlainIssue::from_issue`, so there was never a second call to hit the cache. The only real cost is that `IssueFilter::matches` previously cached per `(filter, issue)` pair; that caching is preserved because `matches` itself remains a `#[turbo_tasks::function]` and now calls `into_trait_ref().await?` once at the top. **`ResolvingIssueWithLocation` eagerly reads inner fields:** Instead of lazily delegating to the inner issue, `stage` and `documentation_link` are copied at construction. This is a minor semantic change (the values are snapshotted rather than live), but in practice these fields are always constant values so it makes no difference. ## Binary size impact (darwin-arm64, compared against #92593) | Metric | #92593 baseline | This PR | Delta | |---|---|---|---| | Unstripped dylib | 124.6 MB | 124.0 MB | **-0.6 MB (-0.5%)** | | Stripped | 84.7 MB | 84.4 MB | **-0.3 MB (-0.4%)** | | Stripped + gzip (npm) | 29.2 MB | 29.1 MB | **-0.1 MB (-0.3%)** | Smaller savings than `ValueDebug` (~2.5 MB stripped) because there are ~48 `Issue` impls vs. one `ValueDebug` impl per every `#[turbo_tasks::value]` type. But real and cumulative. <!-- NEXT_JS_LLM_PR -->
68 lines
2.7 KiB
TypeScript
68 lines
2.7 KiB
TypeScript
/* eslint-env jest */
|
|
|
|
import { isNextStart, nextTestSetup } from 'e2e-utils'
|
|
import { waitForRedbox, getRedboxSource } from 'next-test-utils'
|
|
|
|
describe('Invalid Global CSS', () => {
|
|
const { next, skipped, isTurbopack, isRspack } = nextTestSetup({
|
|
files: __dirname,
|
|
skipStart: isNextStart,
|
|
skipDeployment: true,
|
|
dependencies: { sass: '1.54.0' },
|
|
})
|
|
|
|
if (skipped) {
|
|
return
|
|
}
|
|
|
|
if (isNextStart) {
|
|
it('should fail to build', async () => {
|
|
const { exitCode, cliOutput } = await next.build()
|
|
expect(exitCode).not.toBe(0)
|
|
if (!isTurbopack) {
|
|
expect(cliOutput).toContain('Failed to compile')
|
|
}
|
|
expect(cliOutput).toContain('styles/global.scss')
|
|
expect(cliOutput).toMatch(
|
|
/Please move all first-party global CSS imports.*?pages(\/|\\)_app/
|
|
)
|
|
// Skip: Rspack loaders cannot access module issuer info for location details
|
|
if (!process.env.NEXT_RSPACK) {
|
|
expect(cliOutput).toMatch(/Location:.*pages[\\/]index\.js/)
|
|
}
|
|
})
|
|
} else {
|
|
it('should show a build error', async () => {
|
|
const browser = await next.browser('/')
|
|
|
|
await waitForRedbox(browser)
|
|
const errorSource = await getRedboxSource(browser)
|
|
|
|
if (isTurbopack) {
|
|
expect(errorSource).toMatchInlineSnapshot(`
|
|
"./pages/index.js
|
|
Global CSS cannot be imported from files other than your Custom <App>.
|
|
Due to the Global nature of stylesheets, and to avoid conflicts, Please move all first-party global CSS imports to pages/_app.js. Or convert the import to Component-Level CSS (CSS Modules).
|
|
Location: pages/index.js
|
|
Import path: ../styles/global.scss
|
|
|
|
https://nextjs.org/docs/messages/css-global"
|
|
`)
|
|
} else if (isRspack) {
|
|
expect(errorSource).toMatchInlineSnapshot(`
|
|
"./styles/global.scss
|
|
│ Global CSS cannot be imported from files other than your Custom <App>. Due to the Global nature of stylesheets, and to avoid conflicts, Please move all first-party global CSS imports to pages/_app.js. Or convert the import to Component-Level CSS (CSS Modules).
|
|
│ Read more: https://nextjs.org/docs/messages/css-global"
|
|
`)
|
|
} else {
|
|
expect(errorSource).toMatchInlineSnapshot(`
|
|
"./styles/global.scss
|
|
Global CSS cannot be imported from files other than your Custom <App>. Due to the Global nature of stylesheets, and to avoid conflicts, Please move all first-party global CSS imports to pages/_app.js. Or convert the import to Component-Level CSS (CSS Modules).
|
|
Read more: https://nextjs.org/docs/messages/css-global
|
|
Location: pages/index.js"
|
|
`)
|
|
}
|
|
})
|
|
}
|
|
})
|