Skip to content
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

Add fetcherKey APIs and update fetcher persistence architecture #7704

Merged
merged 9 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/fetcher-persist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@remix-run/dev": minor
"@remix-run/react": minor
---

Add a new `future.v3_fetcherPersist` flag to change the persistence behavior of fetchers. Instead of being immediately cleaned up when unmoutned in the UI, fetchers will persist until they return to an `idle` state ([RFC](https://github.com/remix-run/remix/discussions/7698))

- This is sort of 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
- Keep an eye out for the following specific behavioral changes when opting into this flag and check your app for compatibility:
- 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`).
- 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.
10 changes: 10 additions & 0 deletions docs/components/form.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ The encoding type to use for the form submission.

Defaults to `application/x-www-form-urlencoded`, use `multipart/form-data` for file uploads.

## `navigate`

You can tell the form to skip the navigation and use a [fetcher][use_fetcher] internally by specifying `<Form navigate={false}>`. This is essentially a shorthand for `useFetcher()` + `<fetcher.Form>` where you don't care about the resulting data and only want to kick off a submission and access the pending state via [`useFetchers()`][use_fetchers].

## `fetcherKey`

When using a non-navigating `Form`, you may also optionally specify your own fetcher key to use via `<Form navigate={false} fetcherKey="my-key">`.

### `preventScrollReset`

If you are using [`<ScrollRestoration>`][scroll_restoration_component], this lets you prevent the scroll position from being reset to the top of the window when the form is submitted.
Expand Down Expand Up @@ -133,6 +141,8 @@ See also:
[fullstack_data_flow]: ../discussion/data-flow
[pending_ui]: ../discussion/pending-ui
[form_vs_fetcher]: ../discussion/form-vs-fetcher
[use_fetcher]: ../hooks/use-fetcher
[use_fetchers]: ../hooks/use-fetchers
[fetcher_form]: ../hooks/use-fetcher#fetcherform
[progressive_enhancement]: ../discussion/progressive-enhancement
[use-view-transition-state]: ../hooks//use-view-transition-state
Expand Down
16 changes: 14 additions & 2 deletions docs/file-conventions/remix-config.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ This file has a few build and development configuration options, but does not ac
module.exports = {
appDirectory: "app",
assetsBuildDirectory: "public/build",
future: {
/* any enabled future flags */
},
ignoredRouteFiles: ["**/.*"],
publicPath: "/build/",
routes(defineRoutes) {
Expand Down Expand Up @@ -63,6 +66,14 @@ When using this option and targeting non-Node.js server platforms, you may also
The path to a directory Remix can use for caching things in development,
relative to `remix.config.js`. Defaults to `".cache"`.

## future

The `future` config lets you opt-into future breaking changes via [Future Flags][future-flags]. The following future flags currently exist in Remix v2 and will become the default behavior in Remix v3:

- **`v3_fetcherPersist`**: Change fetcher persistence/cleanup behavior in 2 ways ([RFC][fetcherpersist-rfc]):
- Fetchers are no longer removed on unmount, and remain exposed via `useFetchers` until they return to an `idle` state
- Fetchers that complete while still mounted no longer persist in `useFetchers` since you can access those fetchers via `useFetcher`

## ignoredRouteFiles

This is an array of globs (via [minimatch][minimatch]) that Remix will match to
Expand Down Expand Up @@ -237,12 +248,13 @@ There are a few conventions that Remix uses you should be aware of.
[dilum_sanjaya]: https://twitter.com/DilumSanjaya
[an_awesome_visualization]: https://remix-routing-demo.netlify.app
[remix_dev]: ../other-api/dev#remix-dev
[app_directory]: #appDirectory
[app_directory]: #appdirectory
[css_side_effect_imports]: ../styling/css-imports
[postcss]: https://postcss.org
[tailwind_functions_and_directives]: https://tailwindcss.com/docs/functions-and-directives
[jspm]: https://github.com/jspm/jspm-core
[esbuild_plugins_node_modules_polyfill]: https://npm.im/esbuild-plugins-node-modules-polyfill
[port]: ../other-api/dev#options-1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was unused

[browser-node-builtins-polyfill]: #browsernodebuiltinspolyfill
[server-node-builtins-polyfill]: #servernodebuiltinspolyfill
[future-flags]: ../start/future-flags.md
[fetcherpersist-rfc]: https://github.com/remix-run/remix/discussions/7698
29 changes: 29 additions & 0 deletions docs/hooks/use-fetcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,34 @@ export function SomeComponent() {
}
```

## Options

### `key`

By default, `useFetcher` generate a unique fetcher scoped to that component (however, it may be looked up in [`useFetchers()`][use_fetchers] while in-flight). If you want to identify a fetcher with your own key such that you can access it from elsewhere in your app, you can do that with the `key` option:

```tsx
function AddToBagButton() {
const fetcher = useFetcher({ key: "add-to-bag" });
return <fetcher.Form method="post">...</fetcher.Form>;
}

// Then, up in the header...
function CartCount({ count }) {
const fetcher = useFetcher({ key: "add-to-bag" });
const inFlightCount = Number(
fetcher.formData?.get("quantity") || 0
);
const optimisticCount = count + inFlightCount;
return (
<>
<BagIcon />
<span>{optimisticCount}</span>
</>
);
}
```

## Components

### `fetcher.Form`
Expand Down Expand Up @@ -117,3 +145,4 @@ The form method of the submission.
[network_concurrency_management]: ../discussion/concurrency
[concurrent_mutations_with_use_fetcher]: https://www.youtube.com/watch?v=vTzNpiOk668&list=PLXoynULbYuEDG2wBFSZ66b85EIspy3fy6
[optimistic_ui]: https://www.youtube.com/watch?v=EdB_nj01C80&list=PLXoynULbYuEDG2wBFSZ66b85EIspy3fy6
[use_fetchers]: ./use-fetchers
4 changes: 3 additions & 1 deletion docs/hooks/use-fetchers.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ toc: false

# `useFetchers`

Returns an array of all inflight fetchers. This is useful for components throughout the app that didn't create the fetchers but want to use their submissions to participate in optimistic UI.
Returns an array of all in-flight fetchers. This is useful for components throughout the app that didn't create the fetchers but want to use their submissions to participate in optimistic UI.

```tsx
import { useFetchers } from "@remix-run/react";
Expand All @@ -30,6 +30,7 @@ The fetchers don't contain [`fetcher.Form`][fetcher_form], [`fetcher.submit`][fe
**API**

- [`useFetcher`][use_fetcher]
- [`v3_fetcherPersist`][fetcherpersist]

[fetcher_form]: ./use-fetcher#fetcherform
[fetcher_submit]: ./use-fetcher#fetchersubmitformdata-options
Expand All @@ -39,3 +40,4 @@ The fetchers don't contain [`fetcher.Form`][fetcher_form], [`fetcher.submit`][fe
[form_vs_fetcher]: ../discussion/form-vs-fetcher
[pending_optimistic_ui]: ../discussion/pending-ui
[use_fetcher]: ./use-fetcher
[fetcherpersist]: ../file-conventions/remix-config#future
2 changes: 2 additions & 0 deletions docs/hooks/use-submit.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ Options for the submission, the same as `<Form>` props. All options are optional
- **action**: The href to submit to. Default is the current route path.
- **method**: The HTTP method to use like POST, default is GET.
- **encType**: The encoding type to use for the form submission: `application/x-www-form-urlencoded` or `multipart/form-data`. Default is url encoded.
- **navigate**: Specify `false` to submit using a fetcher instead of performing a navigation
- **fetcherKey**: The fetcher key to use when submitting using a fetcher via `navigate: false`
- **preventScrollReset**: Prevents the scroll position from being reset to the top of the window when the data is submitted. Default is `false`.
- **replace**: Replaces the current entry in the history stack, instead of pushing the new entry. Default is `false`.
- **relative**: Defines relative route resolution behavior. Either `"route"` (relative to the route hierarchy) or `"path"` (relative to the URL).
Expand Down
18 changes: 9 additions & 9 deletions integration/fetcher-layout-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test.beforeAll(async () => {
return (
<div>
<h1>Layout</h1>
<button onClick={invokeFetcher}>Invoke Fetcher</button>
<button id="layout-fetcher" onClick={invokeFetcher}>Invoke Fetcher</button>
{!!fetcher.data && <p id="layout-fetcher-data">{fetcher.data}</p>}
<Outlet />
</div>
Expand Down Expand Up @@ -118,7 +118,7 @@ test.beforeAll(async () => {
return (
<div>
<h1>Layout</h1>
<button onClick={invokeFetcher}>Invoke Fetcher</button>
<button id="layout-fetcher" onClick={invokeFetcher}>Invoke Fetcher</button>
{!!fetcher.data && <p id="layout-fetcher-data">{fetcher.data}</p>}
<Outlet />
</div>
Expand Down Expand Up @@ -194,7 +194,7 @@ test("fetcher calls layout route action when at index route", async ({
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-action");
await app.clickElement("button");
await app.clickElement("#layout-fetcher");
await page.waitForSelector("#layout-fetcher-data");
let dataElement = await app.getElement("#layout-fetcher-data");
expect(dataElement.text()).toBe("layout action data");
Expand All @@ -207,7 +207,7 @@ test("fetcher calls layout route loader when at index route", async ({
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-loader");
await app.clickElement("button");
await app.clickElement("#layout-fetcher");
await page.waitForSelector("#layout-fetcher-data");
let dataElement = await app.getElement("#layout-fetcher-data");
expect(dataElement.text()).toBe("layout loader data");
Expand Down Expand Up @@ -242,26 +242,26 @@ test("fetcher calls layout route action when at paramaterized route", async ({
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-action/foo");
await app.clickElement("button");
await app.clickElement("#layout-fetcher");
await page.waitForSelector("#layout-fetcher-data");
let dataElement = await app.getElement("#layout-fetcher-data");
expect(dataElement.text()).toBe("layout action data");
dataElement = await app.getElement("#child-data");
expect(dataElement.text()).toBe("foo");
});

test("fetcher calls layout route loader when at paramaterized route", async ({
test("fetcher calls layout route loader when at parameterized route", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-loader/foo");
await app.clickElement("button");
await app.clickElement("#layout-fetcher");
await page.waitForSelector("#layout-fetcher-data");
let dataElement = await app.getElement("#layout-fetcher-data");
expect(dataElement.text()).toBe("layout loader data");
});

test("fetcher calls paramaterized route route action", async ({ page }) => {
test("fetcher calls parameterized route route action", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-action/foo");
await app.clickElement("#param-fetcher");
Expand All @@ -272,7 +272,7 @@ test("fetcher calls paramaterized route route action", async ({ page }) => {
expect(dataElement.text()).toBe("foo");
});

test("fetcher calls paramaterized route route loader", async ({ page }) => {
test("fetcher calls parameterized route route loader", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto("/layout-loader/foo");
await app.clickElement("#param-fetcher");
Expand Down
4 changes: 3 additions & 1 deletion packages/remix-dev/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ describe("readConfig", () => {
"entryClientFilePath": Any<String>,
"entryServerFile": "entry.server.tsx",
"entryServerFilePath": Any<String>,
"future": {},
"future": {
"v3_fetcherPersist": false,
},
"mdx": undefined,
"postcss": true,
"publicPath": "/build/",
Expand Down
8 changes: 6 additions & 2 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ type Dev = {
tlsCert?: string;
};

interface FutureConfig {}
interface FutureConfig {
v3_fetcherPersist: boolean;
}

type NodeBuiltinsPolyfillOptions = Pick<
EsbuildPluginsNodeModulesPolyfillOptions,
Expand Down Expand Up @@ -573,7 +575,9 @@ export async function resolveConfig(
// list below, so we can let folks know if they have obsolete flags in their
// config. If we ever convert remix.config.js to a TS file, so we get proper
// typings this won't be necessary anymore.
let future: FutureConfig = {};
let future: FutureConfig = {
v3_fetcherPersist: appConfig.future?.v3_fetcherPersist === true,
};

if (appConfig.future) {
let userFlags = appConfig.future;
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-dev/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@mdx-js/mdx": "^2.3.0",
"@npmcli/package-json": "^4.0.1",
"@remix-run/node": "2.1.0",
"@remix-run/router": "1.10.0",
"@remix-run/router": "1.11.0-pre.0",
"@remix-run/server-runtime": "2.1.0",
"@types/mdx": "^2.0.5",
"@vanilla-extract/integration": "^6.2.0",
Expand Down
5 changes: 4 additions & 1 deletion packages/remix-dev/vite/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { replaceImportSpecifier } from "./replace-import-specifier";
const supportedRemixConfigKeys = [
"appDirectory",
"assetsBuildDirectory",
"future",
"ignoredRouteFiles",
"publicPath",
"routes",
Expand Down Expand Up @@ -245,7 +246,9 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
serverBuildPath,
serverModuleFormat,
relativeAssetsBuildDirectory,
future: {},
future: {
v3_fetcherPersist: options.future?.v3_fetcherPersist === true,
},
};
};

Expand Down
1 change: 1 addition & 0 deletions packages/remix-react/browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
hydrationData,
future: {
v7_normalizeFormMethod: true,
v7_fetcherPersist: window.__remixContext.future.v3_fetcherPersist,
},
});
// @ts-ignore
Expand Down
8 changes: 4 additions & 4 deletions packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1027,10 +1027,10 @@ export function useActionData<T = AppData>(): SerializeFrom<T> | undefined {
*
* @see https://remix.run/hooks/use-fetcher
*/
export function useFetcher<TData = AppData>(): FetcherWithComponents<
SerializeFrom<TData>
> {
return useFetcherRR();
export function useFetcher<TData = AppData>(
opts: Parameters<typeof useFetcherRR>[0] = {}
): FetcherWithComponents<SerializeFrom<TData>> {
return useFetcherRR(opts);
}

// Dead Code Elimination magic for production builds.
Expand Down
4 changes: 3 additions & 1 deletion packages/remix-react/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export interface EntryContext extends RemixContextObject {
staticHandlerContext: StaticHandlerContext;
}

export interface FutureConfig {}
export interface FutureConfig {
v3_fetcherPersist: boolean;
}

export interface AssetsManifest {
entry: {
Expand Down
4 changes: 2 additions & 2 deletions packages/remix-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
"typings": "dist/index.d.ts",
"module": "dist/esm/index.js",
"dependencies": {
"@remix-run/router": "1.10.0",
"@remix-run/router": "1.11.0-pre.0",
"@remix-run/server-runtime": "2.1.0",
"react-router-dom": "6.17.0"
"react-router-dom": "6.18.0-pre.0"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.17.0",
Expand Down
4 changes: 3 additions & 1 deletion packages/remix-server-runtime/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ export interface EntryContext {
serializeError(error: Error): SerializedError;
}

export interface FutureConfig {}
export interface FutureConfig {
v3_fetcherPersist: boolean;
}

export interface AssetsManifest {
entry: {
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-server-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"typings": "dist/index.d.ts",
"module": "dist/esm/index.js",
"dependencies": {
"@remix-run/router": "1.10.0",
"@remix-run/router": "1.11.0-pre.0",
"@types/cookie": "^0.4.1",
"@web3-storage/multipart-parser": "^1.0.0",
"cookie": "^0.4.1",
Expand Down
4 changes: 3 additions & 1 deletion packages/remix-testing/create-remix-stub.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ export function createRemixStub(

if (routerRef.current == null) {
remixContextRef.current = {
future: { ...future },
future: {
v3_fetcherPersist: future?.v3_fetcherPersist === true,
},
manifest: {
routes: {},
entry: { imports: [], module: "" },
Expand Down
4 changes: 2 additions & 2 deletions packages/remix-testing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
"dependencies": {
"@remix-run/node": "2.1.0",
"@remix-run/react": "2.1.0",
"@remix-run/router": "1.10.0",
"react-router-dom": "6.17.0"
"@remix-run/router": "1.11.0-pre.0",
"react-router-dom": "6.18.0-pre.0"
},
"devDependencies": {
"@types/node": "^18.17.1",
Expand Down
4 changes: 4 additions & 0 deletions scripts/bump-router-versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ fi

set -x

cd packages/remix-dev
yarn add -E @remix-run/router@${ROUTER_VERSION}
cd ../..

cd packages/remix-server-runtime
yarn add -E @remix-run/router@${ROUTER_VERSION}
cd ../..
Expand Down
Loading