Allow server-only in server targets and client-only in client components targets to be available#55394
Merged
Merged
Conversation
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Secrets | View in Orca |
Member
Tests Passed |
373984e to
7737a0d
Compare
ztanner
reviewed
Sep 16, 2023
shuding
approved these changes
Sep 18, 2023
shuding
left a comment
Member
There was a problem hiding this comment.
Thanks for the code refactoring!
sokra
reviewed
Sep 18, 2023
| optimize_barrel_exports: None, | ||
| optimize_server_react: None, | ||
| disable_checks: false, | ||
| bundle_target: String::from("default").into(), |
Member
There was a problem hiding this comment.
This should be an enum, shouldn't it?
Contributor
Author
There was a problem hiding this comment.
yes, enum will be better, add a reminder for myself to refactor it later. thanks for suggestion 👍
|
@huozhi @timneutkens Just a big thank you from myself... This unblocks us for Next and so we can proceed. In general, how long does it take for canary to be cut into a new version. However, thank you very much everyone. This has been tested and it works. |
Contributor
Author
|
@barryengineerapart glad to hear it's confirmed working! thanks |
ijjk
pushed a commit
that referenced
this pull request
Sep 18, 2023
We need to disable the default treat `middleware` and `pages/api` as server-only, unless users explictly import "server-only" to poison it. This will avoid the case that when a library is mixing "client-only" API and shared components API in one bundle, and the shared API is used in middleware or `pages/api` that might cause error. See the test case added. Follow up for #55394
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Users want to use
server-onlyto restrict the middleware / app routes / pages api, but now it's failing as we're treating them as different webpack layers, but validating theserver-onlyonly with server components layers.Here we modify the rules a bit to let everyone can use "server-only" for the bundles that targeting server-side.
For next-swc transformer, we introduce the new option
bundleTypewhich only has"server" | "client" | "default"3 values:serverfor server-side targets, like server components, app routes, pages api, middlewareclientfor client components targets such as client components app pages, or page routes under pages directory.defaultfor environment like jest, we don't validate module graph with swc, replaced thedisable_checksintroduced #54891.Refactor a bit webpack-config to adapt to the new rules, after that
server-onlywill be able to used in the server-side targets conventions like middleware andpages/apiFixes #43700
Fixes #54549
Fixes #52833
Closes NEXT-1616
Closes NEXT-1607
Closes NEXT-1385