Remove cache field from action types
#58938
Merged
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
acdlite
requested review from
timneutkens,
ijjk,
shuding,
huozhi,
feedthejim,
ztanner and
wyattjoh
as code owners
The app router state used to be managed by `useReducer`, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the router state outside of React (vercel#56497), each action runs only a single time. So we can simplify some of the logic. Previously, actions that update that cache were passed an empty cache object as part of the action payload. Now, we can instead create these objects on demand in the reducer. (We should do the same for the `mutable` object, but there's a unit test which relies on this implementation detail, so I've left that for a separate PR.)
acdlite
force-pushed
the
remove-cache-field-from-action
branch
from
05e7851
to
c0ea1d2
Compare
Stats from current PRDefault BuildGeneral
Client Bundles (main, webpack)
Legacy Client Bundles (polyfills)
Client Pages
Client Build Manifests
Rendered Page Sizes
Edge SSR bundle Size
Middleware size
Next Runtimes
Diff detailsDiff for page.jsDiff too large to display Diff for 199-HASH.jsDiff too large to display |
Tests Passed |
ztanner
approved these changes
Nov 27, 2023
huozhi
approved these changes
Nov 27, 2023
gnoff
approved these changes
Nov 27, 2023
acdlite
added a commit
to acdlite/next.js
that referenced
this pull request
Dec 4, 2023
Similar in spirit to vercel#58938. The app router reducer state used to be managed by useReducer, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the reducer outside of React (vercel#56497), each action runs only a single time. So we can simplify some of the logic. The purpose of the `mutable` field was so that if an action is processed multiple times, state could be reused between each run; for example, to prevent redundant network fetches. Now that this scenario can no longer happen, we can remove it. I had to update some of the unit tests in navigate-reducer because they were written with the assumption that the reducer was called multiple times. As far as I can tell, most of this behavior is covered by e2e tests anyway, so I don't think it's too risky.
acdlite
added a commit
to acdlite/next.js
that referenced
this pull request
Dec 4, 2023
Similar in spirit to vercel#58938. The app router reducer state used to be managed by useReducer, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the reducer outside of React (vercel#56497), each action runs only a single time. So we can simplify some of the logic. The purpose of the `mutable` field was so that if an action is processed multiple times, state could be reused between each run; for example, to prevent redundant network fetches. Now that this scenario can no longer happen, we can remove it. I had to update some of the unit tests in navigate-reducer because they were written with the assumption that the reducer was called multiple times. As far as I can tell, most of this behavior is covered by e2e tests anyway, so I don't think it's too risky.
acdlite
added a commit
to acdlite/next.js
that referenced
this pull request
Dec 4, 2023
Similar in spirit to vercel#58938. The app router reducer state used to be managed by useReducer, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the reducer outside of React (vercel#56497), each action runs only a single time. So we can simplify some of the logic. The purpose of the `mutable` field was so that if an action is processed multiple times, state could be reused between each run; for example, to prevent redundant network fetches. Now that this scenario can no longer happen, we can remove it. I had to update some of the unit tests in navigate-reducer because they were written with the assumption that the reducer was called multiple times. As far as I can tell, most of this behavior is covered by e2e tests anyway, so I don't think it's too risky.
acdlite
added a commit
that referenced
this pull request
Dec 4, 2023
Similar in spirit to #58938. The app router reducer state used to be managed by useReducer, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the reducer outside of React (#56497), each action runs only a single time. So we can simplify some of the logic. The purpose of the `mutable` field was so that if an action is processed multiple times, state could be reused between each run; for example, to prevent redundant network fetches. Now that this scenario can no longer happen, we can remove it. I had to update some of the unit tests in navigate-reducer because they were written with the assumption that the reducer was called multiple times. As far as I can tell, most of this behavior is covered by e2e tests anyway, so I don't think it's too risky. Closes NEXT-1782
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.
The app router reducer state used to be managed by
useReducer, so it was written to be resilient to rebasing — the same action may be processed multiple times. Now that we've lifted the reducer outside of React (#56497), each action runs only a single time. So we can simplify some of the logic.Previously, actions that update that cache were passed an empty cache object as part of the action payload. Now, we can instead create these objects on demand in the reducer.
(We should do the same for the
mutableobject, but there's a unit test which relies on this implementation detail, so I've left that for a separate PR.)