Skip to content

Comments

Fix: css in next/dynamic component in edge runtime (#64382)#1

Open
MitchLewis930 wants to merge 1 commit intopr_031_beforefrom
pr_031_after
Open

Fix: css in next/dynamic component in edge runtime (#64382)#1
MitchLewis930 wants to merge 1 commit intopr_031_beforefrom
pr_031_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_031

Summary by CodeRabbit

  • New Features

    • Added support for dynamic CSS loading in edge runtime environments.
  • Bug Fixes

    • Unified request handling so non-middleware routes run within the same request context as middleware routes, improving consistency.
  • Tests

    • Added end-to-end test coverage to verify CSS preloading/behavior for pages served in the edge runtime.

### What

Wrap async local storage for all edge runtime routes in adapter 

Basically fixed the case reported in [this
tweet](https://x.com/keegandonley/status/1778538456458854880)

### Why

We're relying on the ALS for dynamic css preloading but we didn't wrap
the ALS for request handlers for edge. So if you have CSS imports in
`next/dynamic` in edge runtime it would break.

Closes NEXT-3085
@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Adapter now ensures non-middleware requests execute within RequestAsyncStorageWrapper.wrap; preload-css switches to a local getExpectedRequestStore function call with a string identifier; an edge-runtime test page and a matching test asserting CSS behavior were added.

Changes

Cohort / File(s) Summary
Server adapter
packages/next/src/server/web/adapter.ts
Non-middleware handler invocations are wrapped with RequestAsyncStorageWrapper.wrap, providing the request via async storage to unify middleware and non-middleware paths.
Dynamic CSS preload
packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx
Replaced destructured require+call with a local typed reference getExpectedRequestStore, calling it with the string 'next/dynamic css' to obtain requestStore.
Edge runtime tests
test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js, test/e2e/app-dir/dynamic-css/index.test.ts
Added an edge-runtime proxy page exporting the existing page plus export const runtime = 'edge', and a test asserting CSS (color and border) for the edge-served page matches expectations.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client as Client
participant Adapter as Adapter\n(packages/next/src/server/web/adapter.ts)
participant AsyncStore as RequestAsyncStorageWrapper\n(RequestAsyncStorageWrapper)
participant Handler as App Handler / Middleware

Client->>Adapter: send Request
Adapter-->>Adapter: detect middleware vs non-middleware path
alt middleware path
    Adapter->>AsyncStore: wrap(req, renderOpts)
    AsyncStore->>Handler: invoke middleware-aware handler
    Handler-->>Client: response
else non-middleware path
    Adapter->>AsyncStore: wrap(req)
    AsyncStore->>Handler: invoke handler with request in storage
    Handler-->>Client: response
end
Note right of AsyncStore: rgba(100,149,237,0.5)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 In tunnels of async and thread,
I wrap each request before it's led.
Edge hops in with stylesheet cheer,
Tests confirm the colors here.
A nimble hop — the path is fed.

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is almost entirely empty, containing only 'PR_031' which provides no meaningful information about the changes, reasoning, or testing. Add a detailed description following the template: explain what the bug was, why it occurred, how the fix works, and confirm tests were added and pass locally.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: addressing CSS handling for next/dynamic components in edge runtime environments.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into pr_031_before

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pr_031_after

No actionable comments were generated in the recent review. 🎉

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b3ead5 and 56d0988.

📒 Files selected for processing (4)
  • packages/next/src/server/web/adapter.ts
  • packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx
  • test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js
  • test/e2e/app-dir/dynamic-css/index.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/next/src/server/web/adapter.ts (1)
packages/next/src/server/async-storage/request-async-storage-wrapper.ts (1)
  • RequestAsyncStorageWrapper (54-139)
packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx (2)
packages/next/src/client/components/request-async-storage.external.ts (1)
  • getExpectedRequestStore (26-32)
packages/next/src/server/web/sandbox/context.ts (1)
  • requestStore (236-238)
🔇 Additional comments (4)
packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx (1)

8-11: LGTM!

The refactor improves type safety with the explicit type annotation and provides a clear context string ('next/dynamic css') for the error message if called outside a request scope. This relies on the corresponding adapter change to wrap non-middleware edge requests with RequestAsyncStorageWrapper.

test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js (1)

1-3: LGTM!

Clean edge runtime test fixture that re-uses the existing SSR page component with the appropriate runtime declaration.

test/e2e/app-dir/dynamic-css/index.test.ts (1)

34-49: LGTM!

Good test coverage for the edge runtime CSS fix. The test appropriately mirrors the existing SSR test structure and validates that the correct CSS is applied.

Minor note: The comment on line 42 references /ssr but applies to /ssr/edge - consider updating to "not loaded in /ssr/edge" for clarity, though this is purely cosmetic.

packages/next/src/server/web/adapter.ts (1)

247-253: Review the purpose of the non-middleware path to determine if missing renderOpts is intentional.

The non-middleware path wraps with only { req: request }, causing reactLoadableManifest to default to {} and assetPrefix to ''. This observation is correct, but requires clarification:

  • The middleware path serves actual middleware for SSR requests and includes full renderOpts
  • The non-middleware path handles edge route handlers (typically API routes), which are dispatched through EdgeRouteModuleWrapper
  • Edge route handlers are fundamentally different from SSR rendering paths and typically don't render dynamic components requiring CSS preloading

The PreloadCss component has defensive checks (if (requestStore.reactLoadableManifest && moduleIds) and if (!manifest[key]) continue), and empty manifest objects are used intentionally elsewhere in the codebase (e.g., load-default-error-components.ts).

Verify whether edge route handlers in this codebase ever render dynamic components that would require CSS preloading. If they don't, the minimal wrapping is intentional; if they do, the missing renderOpts should be added.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@MitchLewis930
Copy link
Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

✅ Actions performed

Full review triggered.

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