-
Notifications
You must be signed in to change notification settings - Fork 27k
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
fix useSelectedLayoutSegment's support for parallel routes #60912
Merged
ztanner
merged 18 commits into
vercel:canary
from
williamli:william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the
Jan 23, 2024
Merged
fix useSelectedLayoutSegment's support for parallel routes #60912
ztanner
merged 18 commits into
vercel:canary
from
williamli:william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the
Jan 23, 2024
Conversation
This file contains 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
Stats from current PRDefault BuildGeneral Overall increase
|
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
buildDuration | 11.7s | 11.5s | N/A |
buildDurationCached | 6s | 4.9s | N/A |
nodeModulesSize | 200 MB | 200 MB | |
nextStartRea..uration (ms) | 423ms | 432ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
193.HASH.js gzip | 181 B | 182 B | N/A |
3f784ff6-HASH.js gzip | 53.4 kB | 53.4 kB | ✓ |
433-HASH.js gzip | 29 kB | 29 kB | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 241 B | 241 B | ✓ |
main-HASH.js gzip | 31.8 kB | 31.8 kB | N/A |
webpack-HASH.js gzip | 1.7 kB | 1.7 kB | N/A |
Overall change | 98.8 kB | 98.8 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
_app-HASH.js gzip | 194 B | 195 B | N/A |
_error-HASH.js gzip | 183 B | 181 B | N/A |
amp-HASH.js gzip | 504 B | 502 B | N/A |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | N/A |
edge-ssr-HASH.js gzip | 255 B | 253 B | N/A |
head-HASH.js gzip | 350 B | 349 B | N/A |
hooks-HASH.js gzip | 369 B | 369 B | ✓ |
image-HASH.js gzip | 4.18 kB | 4.18 kB | N/A |
index-HASH.js gzip | 255 B | 256 B | N/A |
link-HASH.js gzip | 2.61 kB | 2.61 kB | ✓ |
routerDirect..HASH.js gzip | 312 B | 311 B | N/A |
script-HASH.js gzip | 385 B | 383 B | N/A |
withRouter-HASH.js gzip | 307 B | 308 B | N/A |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 3.4 kB | 3.4 kB | ✓ |
Client Build Manifests
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
_buildManifest.js gzip | 484 B | 485 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
index.html gzip | 528 B | 525 B | N/A |
link.html gzip | 540 B | 538 B | N/A |
withRouter.html gzip | 524 B | 522 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
edge-ssr.js gzip | 94 kB | 94 kB | N/A |
page.js gzip | 149 kB | 149 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 626 B | 626 B | ✓ |
middleware-r..fest.js gzip | 151 B | 151 B | ✓ |
middleware.js gzip | 37.5 kB | 37.5 kB | N/A |
edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
Overall change | 2.7 kB | 2.7 kB | ✓ |
Next Runtimes
vercel/next.js canary | williamli/next.js william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 169 kB | 169 kB | ✓ |
app-page-exp..prod.js gzip | 95.6 kB | 95.6 kB | ✓ |
app-page-tur..prod.js gzip | 96.3 kB | 96.3 kB | ✓ |
app-page-tur..prod.js gzip | 90.9 kB | 90.9 kB | ✓ |
app-page.run...dev.js gzip | 142 kB | 142 kB | ✓ |
app-page.run..prod.js gzip | 90.2 kB | 90.2 kB | ✓ |
app-route-ex...dev.js gzip | 24.2 kB | 24.2 kB | ✓ |
app-route-ex..prod.js gzip | 16.9 kB | 16.9 kB | ✓ |
app-route-tu..prod.js gzip | 16.9 kB | 16.9 kB | ✓ |
app-route-tu..prod.js gzip | 16.4 kB | 16.4 kB | ✓ |
app-route.ru...dev.js gzip | 23.6 kB | 23.6 kB | ✓ |
app-route.ru..prod.js gzip | 16.4 kB | 16.4 kB | ✓ |
pages-api-tu..prod.js gzip | 9.41 kB | 9.41 kB | ✓ |
pages-api.ru...dev.js gzip | 9.68 kB | 9.68 kB | ✓ |
pages-api.ru..prod.js gzip | 9.4 kB | 9.4 kB | ✓ |
pages-turbo...prod.js gzip | 22 kB | 22 kB | ✓ |
pages.runtim...dev.js gzip | 22.6 kB | 22.6 kB | ✓ |
pages.runtim..prod.js gzip | 22 kB | 22 kB | ✓ |
server.runti..prod.js gzip | 49.7 kB | 49.7 kB | ✓ |
Overall change | 944 kB | 944 kB | ✓ |
Diff details
Diff for page.js
Diff too large to display
Diff for 433-HASH.js
Diff too large to display
Tests Passed |
…rted by getSelectedLayoutSegmentPath
williamli
requested review from
timneutkens,
ijjk,
shuding,
huozhi,
a team,
feedthejim,
ztanner and
wyattjoh
as code owners
January 20, 2024 08:31
williamli
force-pushed
the
william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the
branch
from
January 21, 2024 07:31
b59d6b5
to
323881f
Compare
…-does-not-return-the-name-of-the
ztanner
reviewed
Jan 22, 2024
test/e2e/app-dir/parallel-routes-use-selected-layout-segment/app/globals.css
Outdated
Show resolved
Hide resolved
...allel-routes-use-selected-layout-segment/parallel-routes-use-selected-layout-segment.test.ts
Outdated
Show resolved
Hide resolved
test/e2e/app-dir/parallel-routes-use-selected-layout-segment/app/layout.tsx
Outdated
Show resolved
Hide resolved
…arallel-routes-use-selected-layout-segment.test.ts Co-authored-by: Zack Tanner <zacktanner@gmail.com>
williamli
force-pushed
the
william/next-2173-useselectedlayoutsegment-does-not-return-the-name-of-the
branch
from
January 23, 2024 03:17
75ce183
to
6aa0e34
Compare
…-does-not-return-the-name-of-the
…-does-not-return-the-name-of-the
…-does-not-return-the-name-of-the
ztanner
approved these changes
Jan 23, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
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.
fixes NEXT-2173
Fixes #59968
TODOs
useSelectedLayoutSegment
to support parallel routes (see "What")useSelectedLayoutSegments
to see if it is affectedWhat?
useSelectedLayoutSegment
does not return the name of the active state of parallel route slots.Expected Behaviour
According to https://nextjs.org/docs/app/building-your-application/routing/parallel-routes#useselectedlayoutsegments
👉🏽 We should update the docs to explain
null
and DEFAULT result as well.According to the API reference for useSelectedLayoutSegment:
Current Behaviour
Currently a string "children" is returned for everything inside a parallel route with active state and
__DEFAULT__
is returned if there is no active state for the parallel route (since thedefault.tsx
is loaded).null
is returned when thedefault.tsx
is not loaded (possibly caused by another bug, see test case 5).Reproduction
GitHub Repo is created based on the example provided in Next.js docs for using
useSelectedLayoutSegment
with Parallel Routes.Test Cases
children
(expected value should belogin
)children
(expected value should belogin
)children
(expected value should bereset
)children
(expected value should belogin
)children
(expected value should bewithEmail
)children
(expected value should belogin
)If you hard nav to (/app/@auth/reset/withEmail) https://next-2173.vercel.app/reset/withEmail, you get an unexpected result due to possibly another bug:navSegment isnull
on the deployed (Vercel) version, the navSlot is not loadednavSegment is__DEFAULT__
on local dev, the navSlot loads/app/@nav/default.tsx
.Why?
In
packages/next/src/client/components/navigation.ts
,getSelectedLayoutSegmentPath
is called and returns the correct segmentPath for parallel routes (even though there is a TODO comment indicating this function needs to be updated to handle parallel routes) butuseSelectedLayoutSegment
failed to return the correct segment when a parallelRouteKey is provided.How?
useSelectedLayoutSegment
is updated to return selectedLayoutSegments[0] for non parallel routes (original logic), but it will return the last segments for parallel routes (or null if nothing is active).