-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Stabilize shouldCallHandler APIs #14592
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
Conversation
🦋 Changeset detectedLatest commit: 83bd9e6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // Single fetch revalidates by default, so override the RR default value which | ||
| // matches the multi-fetch behavior with `true` | ||
| if (ssr && route.shouldRevalidate) { | ||
| let fn = route.shouldRevalidate; | ||
| return (opts: ShouldRevalidateFunctionArgs) => | ||
| fn({ ...opts, defaultShouldRevalidate: true }); | ||
| } |
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.
This is not needed anymore since this was the old hacky solution when we only had shouldLoad. This behavior should be handled by this code now:
react-router/packages/react-router/lib/dom/ssr/single-fetch.tsx
Lines 387 to 391 in a947b03
| let defaultShouldRevalidate = | |
| !m.unstable_shouldRevalidateArgs || | |
| m.unstable_shouldRevalidateArgs.actionStatus == null || | |
| m.unstable_shouldRevalidateArgs.actionStatus < 400; | |
| let shouldCall = m.unstable_shouldCallHandler(defaultShouldRevalidate); |
This is actually causing a not-yet-reported bug where if you have a shouldRevalidate in single fetch, the value of defaultShouldRevalidate is incorrect on acton 4xx/5xx responses. Added a new E2E test to this PR to confirm that fix.
|
🤖 Hello there, We just published version Thanks! |
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [react-router](https://github.com/remix-run/react-router) ([source](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router)) | [`7.9.6` -> `7.10.0`](https://renovatebot.com/diffs/npm/react-router/7.9.6/7.10.0) |  |  | --- ### Release Notes <details> <summary>remix-run/react-router (react-router)</summary> ### [`v7.10.0`](https://github.com/remix-run/react-router/blob/HEAD/packages/react-router/CHANGELOG.md#7100) [Compare Source](https://github.com/remix-run/react-router/compare/[email protected]@7.10.0) ##### Minor Changes - Stabilize `fetcher.reset()` ([#​14545](remix-run/react-router#14545)) -⚠️ This is a breaking change if you have begun using `fetcher.unstable_reset()` - Stabilize the `dataStrategy` `match.shouldRevalidateArgs`/`match.shouldCallHandler()` APIs. ([#​14592](remix-run/react-router#14592)) - The `match.shouldLoad` API is now marked deprecated in favor of these more powerful alternatives - If you're using this API in a custom `dataStrategy` today, you can swap to the new API at your convenience: ```tsx // Before const matchesToLoad = matches.filter((m) => m.shouldLoad); // After const matchesToLoad = matches.filter((m) => m.shouldCallHandler()); ``` - `match.shouldRevalidateArgs` is the argument that will be passed to the route `shouldRevaliate` function - Combined with the parameter accepted by `match.shouldCallHandler`, you can define a custom revalidation behavior for your `dataStrategy`: ```tsx const matchesToLoad = matches.filter((m) => { const defaultShouldRevalidate = customRevalidationBehavior( match.shouldRevalidateArgs, ); return m.shouldCallHandler(defaultShouldRevalidate); // The argument here will override the internal `defaultShouldRevalidate` value }); ``` ##### Patch Changes - Fix a Framework Mode bug where the `defaultShouldRevalidate` parameter to `shouldRevalidate` would not be correct after `action` returned a 4xx/5xx response (`true` when it should have been `false`) ([#​14592](remix-run/react-router#14592)) - If your `shouldRevalidate` function relied on that parameter, you may have seen unintended revalidations - Fix `fetcher.submit` failing with plain objects containing a `tagName` property ([#​14534](remix-run/react-router#14534)) - \[UNSTABLE] Add `unstable_pattern` to the parameters for client side `unstable_onError`, refactor how it's called by `RouterProvider` to avoid potential strict mode issues ([#​14573](remix-run/react-router#14573)) - Add new `unstable_useTransitions` flag to routers to give users control over the usage of [`React.startTransition`](https://react.dev/reference/react/startTransition) and [`React.useOptimistic`](https://react.dev/reference/react/useOptimistic). ([#​14524](remix-run/react-router#14524)) - Framework Mode + Data Mode: - `<HydratedRouter unstable_transition>`/`<RouterProvider unstable_transition>` - When left unset (current default behavior) - Router state updates are wrapped in `React.startTransition` -⚠️ This can lead to buggy behaviors if you are wrapping your own navigations/fetchers in `React.startTransition` - You should set the flag to `true` if you run into this scenario to get the enhanced `useOptimistic` behavior (requires React 19) - When set to `true` - Router state updates remain wrapped in `React.startTransition` (as they are without the flag) - `Link`/`Form` navigations will be wrapped in `React.startTransition` - A subset of router state info will be surfaced to the UI *during* navigations via `React.useOptimistic` (i.e., `useNavigation()`, `useFetchers()`, etc.) -⚠️ This is a React 19 API so you must also be React 19 to opt into this flag for Framework/Data Mode - When set to `false` - The router will not leverage `React.startTransition` or `React.useOptimistic` on any navigations or state changes - Declarative Mode - `<BrowserRouter unstable_useTransitions>` - When left unset - Router state updates are wrapped in `React.startTransition` - When set to `true` - Router state updates remain wrapped in `React.startTransition` (as they are without the flag) - `Link`/`Form` navigations will be wrapped in `React.startTransition` - When set to `false` - the router will not leverage `React.startTransition` on any navigations or state changes - Fix the promise returned from `useNavigate` in Framework/Data Mode so that it properly tracks the duration of `popstate` navigations (i.e., `navigate(-1)`) ([#​14524](remix-run/react-router#14524)) - Fix internal type error in useRoute types that surfaces when skipLibCheck is disabled ([#​14577](remix-run/react-router#14577)) - Preserve `statusText` on the `ErrorResponse` instance when throwing `data()` from a route handler ([#​14555](remix-run/react-router#14555)) - Optimize href() to avoid backtracking regex on splat ([#​14329](remix-run/react-router#14329)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi4yNy4xIiwidXBkYXRlZEluVmVyIjoiNDIuMjcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://git.foxden.network/foxCaves/foxCaves/pulls/13 Co-authored-by: Renovate <[email protected]> Co-committed-by: Renovate <[email protected]>
These are lower level advanced APIs available to Data Mode users with custom
dataStrategyimplementations.A while back we realized that the
match.shouldLoadAPI we had wasn't completely sufficient fordataStrategyimplementations to fully override the default revalidation behavior as we wanted to do in single fetch. We could get pretty close but once we did the don't revalidate on 4xx actions we couldn't quite get ll the way there, which is why this API was developed.It's been in use for single fetch for quite some time now, and will be an important part of call-site revalidation opt-out working right in single fetch, so it feels like time to stabilize and deprecate
match.shouldLoad, and then plan on droppingshouldLoadin v8.This also fixes a yet-unreported bug where the
defaultShouldRevalidateparameter toshouldRevalidatewould not be correct afteractionreturned a 4xx/5xx response (truewhen it should have beenfalse)