remove optimistic navigation behavior when prefetch is false #58413
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
ztanner
changed the title
ztanner
requested review from
timneutkens,
ijjk,
shuding,
huozhi,
feedthejim and
wyattjoh
as code owners
feedthejim
reviewed
Nov 14, 2023
feedthejim
reviewed
Nov 14, 2023
packages/next/src/client/components/router-reducer/fill-cache-with-data-property.ts
Outdated
Show resolved
Hide resolved
feedthejim
reviewed
Nov 14, 2023
feedthejim
reviewed
Nov 14, 2023
timneutkens
reviewed
Nov 14, 2023
timneutkens
reviewed
Nov 14, 2023
packages/next/src/client/components/router-reducer/create-optimistic-tree.ts
Outdated
Show resolved
Hide resolved
ijjk
added
area: tests
created-by: Next.js team
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 link-HASH.js@@ -1,7 +1,7 @@
(self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
[644],
{
- /***/ 1794: /***/ function (
+ /***/ 8959: /***/ function (
__unused_webpack_module,
__unused_webpack_exports,
__webpack_require__
@@ -9,7 +9,7 @@
(window.__NEXT_P = window.__NEXT_P || []).push([
"/link",
function () {
- return __webpack_require__(1921);
+ return __webpack_require__(9877);
},
]);
if (false) {
@@ -18,7 +18,7 @@
/***/
},
- /***/ 172: /***/ function (module, exports) {
+ /***/ 91: /***/ function (module, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", {
@@ -100,7 +100,7 @@
/***/
},
- /***/ 4847: /***/ function (module, exports, __webpack_require__) {
+ /***/ 2976: /***/ function (module, exports, __webpack_require__) {
"use strict";
Object.defineProperty(exports, "__esModule", {
@@ -112,7 +112,7 @@
return getDomainLocale;
},
});
- const _normalizetrailingslash = __webpack_require__(2003);
+ const _normalizetrailingslash = __webpack_require__(9076);
const basePath =
/* unused pure expression or super */ null && (false || "");
function getDomainLocale(path, locale, locales, domainLocales) {
@@ -136,7 +136,7 @@
/***/
},
- /***/ 7414: /***/ function (module, exports, __webpack_require__) {
+ /***/ 106: /***/ function (module, exports, __webpack_require__) {
"use strict";
/* __next_internal_client_entry_do_not_use__ cjs */
Object.defineProperty(exports, "__esModule", {
@@ -152,17 +152,17 @@
const _react = /*#__PURE__*/ _interop_require_default._(
__webpack_require__(959)
);
- const _resolvehref = __webpack_require__(279);
- const _islocalurl = __webpack_require__(7377);
- const _formaturl = __webpack_require__(9616);
- const _utils = __webpack_require__(3869);
- const _addlocale = __webpack_require__(5752);
- const _routercontextsharedruntime = __webpack_require__(1840);
- const _approutercontextsharedruntime = __webpack_require__(3057);
- const _useintersection = __webpack_require__(9285);
- const _getdomainlocale = __webpack_require__(4847);
- const _addbasepath = __webpack_require__(5853);
- const _routerreducertypes = __webpack_require__(172);
+ const _resolvehref = __webpack_require__(8873);
+ const _islocalurl = __webpack_require__(1023);
+ const _formaturl = __webpack_require__(3482);
+ const _utils = __webpack_require__(4597);
+ const _addlocale = __webpack_require__(6845);
+ const _routercontextsharedruntime = __webpack_require__(1049);
+ const _approutercontextsharedruntime = __webpack_require__(4095);
+ const _useintersection = __webpack_require__(4991);
+ const _getdomainlocale = __webpack_require__(2976);
+ const _addbasepath = __webpack_require__(7677);
+ const _routerreducertypes = __webpack_require__(91);
const prefetched = new Set();
function prefetch(router, href, as, options, appOptions, isAppRouter) {
if (false) {
@@ -221,8 +221,7 @@
shallow,
scroll,
locale,
- isAppRouter,
- prefetchEnabled
+ isAppRouter
) {
const { nodeName } = e.currentTarget;
// anchors inside an svg have a lowercase nodeName
@@ -247,7 +246,6 @@
});
} else {
router[replace ? "replace" : "push"](as || href, {
- forceOptimisticNavigation: !prefetchEnabled,
scroll: routerScroll,
});
}
@@ -443,8 +441,7 @@
shallow,
scroll,
locale,
- isAppRouter,
- prefetchEnabled
+ isAppRouter
);
},
onMouseEnter(e) {
@@ -581,7 +578,7 @@
/***/
},
- /***/ 9285: /***/ function (module, exports, __webpack_require__) {
+ /***/ 4991: /***/ function (module, exports, __webpack_require__) {
"use strict";
Object.defineProperty(exports, "__esModule", {
@@ -594,7 +591,7 @@
},
});
const _react = __webpack_require__(959);
- const _requestidlecallback = __webpack_require__(5781);
+ const _requestidlecallback = __webpack_require__(3136);
const hasIntersectionObserver =
typeof IntersectionObserver === "function";
const observers = new Map();
@@ -707,7 +704,7 @@
/***/
},
- /***/ 1921: /***/ function (
+ /***/ 9877: /***/ function (
__unused_webpack_module,
__webpack_exports__,
__webpack_require__
@@ -723,7 +720,7 @@
/* harmony import */ var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__ =
__webpack_require__(1527);
/* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1__ =
- __webpack_require__(3639);
+ __webpack_require__(2075);
/* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1___default =
/*#__PURE__*/ __webpack_require__.n(
next_link__WEBPACK_IMPORTED_MODULE_1__
@@ -754,12 +751,12 @@
/***/
},
- /***/ 3639: /***/ function (
+ /***/ 2075: /***/ function (
module,
__unused_webpack_exports,
__webpack_require__
) {
- module.exports = __webpack_require__(7414);
+ module.exports = __webpack_require__(106);
/***/
},
@@ -770,7 +767,7 @@
return __webpack_require__((__webpack_require__.s = moduleId));
};
/******/ __webpack_require__.O(0, [774, 888, 179], function () {
- return __webpack_exec__(1794);
+ return __webpack_exec__(8959);
});
/******/ var __webpack_exports__ = __webpack_require__.O();
/******/ _N_E = __webpack_exports__;Diff for 199-HASH.jsDiff too large to display |
ztanner
changed the title
timneutkens
reviewed
Nov 15, 2023
packages/next/src/client/components/router-reducer/fill-cache-with-data-property.ts
Outdated
Show resolved
Hide resolved
…with-data-property.ts Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
timneutkens
approved these changes
Nov 16, 2023
amannn
reviewed
Nov 28, 2023
| @@ -69,7 +69,6 @@ export type CacheNode = | |||
|
|
|||
| export interface NavigateOptions { | |||
| /** @internal */ | |||
There was a problem hiding this comment.
I think this should have been removed too, I've opened a PR here: #59001
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.
What?
When navigating between pages (via
prefetch: false) within a dynamic segment, the shared layout is re-rendered. This can cause unexpected behavior like layout data changing when navigating between child segments.Why?
When prefetch is false, we're currently opting into an "optimistic navigation" codepath, which will optimistically render layout-routers up to the point where data is missing, while kicking off data fetches. It attempts to determine where refetching needs to happen by traversing the router cache nodes and checking where data is missing. However, it locates these cache nodes by using "segments" obtained by deconstructing the URL, which won't accurately contain dynamic segment data. For ex,
/enwhich corresponds with/app/[lang]/page.tsxwill have a cache node key oflang|en|d, noten. Similarly, the optimistic tree that gets constructed will also be incorrect, since it uses the URL segment.How?
My initial fix was to match the dynamic segment against the segment constructed by the URL. But after discussion with @sebmarkbage and the team, it seems more correct to remove the optimistic case all together as there's no guarantee that the url will actually match to that segment.
Fixes #50670