Skip to content

Commit

Permalink
Updates
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Oct 20, 2023
1 parent 52b3e40 commit 65d6b8c
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 28 deletions.
7 changes: 4 additions & 3 deletions .changeset/fetcher-persistence.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
Fix the persistence behavior of fetchers so that they don't get cleaned up on `fetcher` unmount, but instead get cleaned up on fetcher completion (which may be after the fetcher unmounts in the UI) ([RFC](https://github.com/remix-run/remix/discussions/7698))

- This is a long-standing bug fix as the `useFetchers()` API was always supposed to only reflect **in-flight** fetcher information for pending/optimistic UI
- It was not intended to reflect fetcher data or hang onto fetchers after they returned to an idle state
- It was not intended to reflect fetcher data or hang onto fetchers after they returned to an `idle` state
- To do this we've re-architected things a bit and now it's the `react-router-dom` layer that holds stateful fetcher data to expose via `useFetcher()`
- The `router` now only knows about in-flight fetchers - they do not exist in `state.fetchers` until a `fetch()` call is made, and they are removed when it returns to `idle` (and the data is handed off to the React layer)
- **Warning:** This has two potential "breaking bug" side-effects for your application:
- Fetchers that previously unmounted _while in-flight_ will not be immediately aborted and will instead be cleaned up once they return to `idle`. They will remain exposed via `useFetchers` while in-flight so you can still access pending/optimistic data after unmount.
- **Warning:** This has a few potential "breaking bug" side-effects for your application:
- `useFetchers()` longer exposes the `data` field because it now only represents in-flight fetchers, and thus it does not reflect fetchers that have completed and have data
- Fetchers that previously unmounted _while in-flight_ will not be immediately aborted and will instead be cleaned up once they return to an `idle` state. They will remain exposed via `useFetchers` while in-flight so you can still access pending/optimistic data after unmount.
- Fetchers that complete while still mounted will no longer appear in `useFetchers()` - they served effectively no purpose in there since you can access the data via `useFetcher().data`)
3 changes: 1 addition & 2 deletions .changeset/fix-type-bug.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
"@remix-run/router": patch
---

- Remove the internal `router.getFetcher` API
- Fix `router.deleteFetcher` type definition which incorrectly specified `key` as an optional parameter
Fix `router.deleteFetcher` type definition which incorrectly specified `key` as an optional parameter
5 changes: 5 additions & 0 deletions .changeset/remove-get-fetcher.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/router": minor
---

Remove the internal `router.getFetcher` API
27 changes: 15 additions & 12 deletions packages/react-router-dom/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export { ViewTransitionContext as UNSAFE_ViewTransitionContext };

// TODO: (v7) Change the useFetcher data from `any` to `unknown`
type FetchersContextObject = {
data: Map<string, any>;
fetcherData: Map<string, any>;
register: (key: string) => void;
unregister: (key: string) => void;
};
Expand Down Expand Up @@ -1286,7 +1286,7 @@ function useFetcherDataLayer() {

let fetcherContext = React.useMemo<FetchersContextObject>(
() => ({
data: fetcherData.current,
fetcherData: fetcherData.current,
register: registerFetcher,
unregister: unregisterFetcher,
}),
Expand Down Expand Up @@ -1582,30 +1582,33 @@ export function useFetcher<TData = any>({
}: { key?: string } = {}): FetcherWithComponents<TData> {
let { router } = useDataRouterContext(DataRouterHook.UseFetcher);
let state = useDataRouterState(DataRouterStateHook.UseFetcher);
let fetchersCtx = React.useContext(FetchersContext);
let fetchersContext = React.useContext(FetchersContext);
let route = React.useContext(RouteContext);
let routeId = route.matches[route.matches.length - 1]?.route.id;
let [fetcherKey, setFetcherKey] = React.useState<string>(key || "");
if (!fetcherKey) {
setFetcherKey(getUniqueFetcherId());
}

invariant(fetchersCtx, `useFetcher must be used inside a FetchersContext`);
invariant(
fetchersContext,
`useFetcher must be used inside a FetchersContext`
);
invariant(route, `useFetcher must be used inside a RouteContext`);
invariant(
routeId != null,
`useFetcher can only be used on routes that contain a unique "id"`
);

let { data, register, unregister } = fetchersCtx;
let { fetcherData, register, unregister } = fetchersContext;

// Register/deregister with FetchersContext
React.useEffect(() => {
register(fetcherKey);
return () => unregister(fetcherKey);
}, [fetcherKey, register, unregister]);

// Fetcher additions
// Fetcher additions (load)
let load = React.useCallback(
(href: string) => {
invariant(routeId, `fetcher.load routeId unavailable`);
Expand All @@ -1614,6 +1617,7 @@ export function useFetcher<TData = any>({
[fetcherKey, routeId, router]
);

// Fetcher additions (submit)
let submitImpl = useSubmit();
let submit = React.useCallback<FetcherSubmitFunction>(
(target, opts) => {
Expand All @@ -1625,7 +1629,6 @@ export function useFetcher<TData = any>({
},
[fetcherKey, submitImpl]
);

let Form = React.useMemo(() => {
let FetcherForm = React.forwardRef<HTMLFormElement, FetcherFormProps>(
(props, ref) => {
Expand All @@ -1646,18 +1649,18 @@ export function useFetcher<TData = any>({
return FetcherForm;
}, [fetcherKey, submit]);

// Exposed stateful fetcher with data from FetchersContext
let data = fetcherData.get(fetcherKey);
return React.useMemo(() => {
// Prefer the fetcher from `state` not `router.state` since DataRouterContext
// is memoized so this ensures we update on fetcher state updates
let fetcher = fetcherKey
? state.fetchers.get(fetcherKey) || IDLE_FETCHER
: IDLE_FETCHER;
let fetcher = state.fetchers.get(fetcherKey) || IDLE_FETCHER;
return {
...fetcher,
Form,
submit,
load,
...fetcher,
data: data.get(fetcherKey),
data,
};
}, [Form, data, fetcherKey, load, state.fetchers, submit]);
}
Expand Down
31 changes: 20 additions & 11 deletions packages/react-router-dom/server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
UNSAFE_DataRouterContext as DataRouterContext,
UNSAFE_DataRouterStateContext as DataRouterStateContext,
UNSAFE_ViewTransitionContext as ViewTransitionContext,
UNSAFE_FetchersContext as FetchersContext,
} from "react-router-dom";

export interface StaticRouterProps {
Expand Down Expand Up @@ -132,17 +133,25 @@ export function StaticRouterProvider({
<>
<DataRouterContext.Provider value={dataRouterContext}>
<DataRouterStateContext.Provider value={state}>
<ViewTransitionContext.Provider value={{ isTransitioning: false }}>
<Router
basename={dataRouterContext.basename}
location={state.location}
navigationType={state.historyAction}
navigator={dataRouterContext.navigator}
static={dataRouterContext.static}
>
<DataRoutes routes={router.routes} state={state} />
</Router>
</ViewTransitionContext.Provider>
<FetchersContext.Provider
value={{
fetcherData: new Map<string, any>(),
register: () => {},
unregister: () => {},
}}
>
<ViewTransitionContext.Provider value={{ isTransitioning: false }}>
<Router
basename={dataRouterContext.basename}
location={state.location}
navigationType={state.historyAction}
navigator={dataRouterContext.navigator}
static={dataRouterContext.static}
>
<DataRoutes routes={router.routes} state={state} />
</Router>
</ViewTransitionContext.Provider>
</FetchersContext.Provider>
</DataRouterStateContext.Provider>
</DataRouterContext.Provider>
{hydrateScript ? (
Expand Down

0 comments on commit 65d6b8c

Please sign in to comment.