Add better control over submission serialization #10342

brophdawg11 · 2023-04-13T20:31:49Z

~~Depends on #10336~~

Closes part of #10324 (does not yet handle call-site/override actions - that will be a separate PR):

Adds support for useSubmit()(obj, { encType: null }) to opt out of serialization for raw payloads. These are then passed to the action via the payload parameter. Same goes for fetcher.submit.
Add support for opt-in serialization of payloads for encType:application/json and encType:text/plain

See the changesets for examples.

changeset-bot · 2023-04-13T20:31:54Z

🦋 Changeset detected

Latest commit: d8dc40a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@remix-run/router	Minor
react-router-dom	Minor
react-router	Minor
react-router-dom-v5-compat	Minor
react-router-native	Minor

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

brophdawg11 · 2023-04-14T20:25:14Z

packages/react-router-dom/dom.ts

+  | FormData
+  | URLSearchParams
+  | { [name: string]: string }
+  | NonNullable<unknown> // Raw payload submissions


We can now submit anything other than undefined as our payload

@ryanflorence - Mark brought up a good point that this could potentially open up a footgun for users thinking anything can be serialized into FormData. With encType:null we can accept anything (and rule out undefined for clarity).

But without encType not everything can be encoded into FormData

// Technically arrays are fine and serialize into formData as a map of index => value submit([1,2,3]) // but strings will blow up without any type hints submit("plain text") // as will nested objects and other complex structures submit({ parent: { child: { key: "value" } } })

Right now the plan is to handle any advanced type inference in a new PR so we can tackle it there based on what we decide. We could also just log a warning if you submit a non-FormData serializable value?

brophdawg11 · 2023-04-14T20:45:42Z

packages/router/router.ts

+    | { formData: FormData; payload?: undefined }
+    | { formData?: undefined; payload: any }


formData/payload are mutually exclusive on the navigation- you can only include one in a call to router.navigate

brophdawg11 · 2023-04-14T20:47:27Z

packages/router/router.ts

+    if (formData) {
+      if (formEncType === "application/x-www-form-urlencoded") {
+        // Content-Type is inferred (https://fetch.spec.whatwg.org/#dom-request)
+        init.body = convertFormDataToSearchParams(formData);
+      } else {
+        // Content-Type is inferred (https://fetch.spec.whatwg.org/#dom-request)
+        init.body = formData;
+      }
+    } else if (payload != null) {
+      if (formEncType === "application/x-www-form-urlencoded") {
+        let payloadFormData = new FormData();
+
+        if (payload instanceof URLSearchParams) {
+          for (let [name, value] of payload) {
+            payloadFormData.append(name, value);
+          }
+        } else if (payload != null) {
+          for (let name of Object.keys(payload)) {
+            // @ts-expect-error
+            payloadFormData.append(name, payload[name]);
+          }
+        }
+
+        // Content-Type is inferred (https://fetch.spec.whatwg.org/#dom-request)
+        init.body = convertFormDataToSearchParams(payloadFormData);
+      } else if (formEncType === "application/json") {
+        init.headers = new Headers({ "Content-Type": formEncType });
+        init.body = JSON.stringify(payload);
+      } else if (formEncType === "text/plain") {
+        init.headers = new Headers({
+          "Content-Type": formEncType,
+        });
+        init.body =
+          typeof payload === "string" ? payload : JSON.stringify(payload);
+      }
+    }


Serialize the incoming formData or payload onto the request if we know how

@ryanflorence Tagging for review - new router serialization based on content-type

docs/hooks/use-submit.md

markdalgleish · 2023-04-17T20:29:16Z

docs/hooks/use-submit.md

+submit(obj, { encType: "text/plain" }); // -> request.text()
+```
+
+<docs-warn>In future versions of React Router, the default behavior will not serialize raw JSON payloads. If you are submitting raw JSON today it's recommended to specify an explicit `encType`.</docs-warn>


Have we talked about making this a future flag?

Ryan and I talked through this a good bit and landed on a heuristic that I need to get added to our docs and specifically this diagram.

A future flag is needed when there is no way to opt-in to the new (breaking) behavior at the call-site (locally), so we need to give them a config (global) opt-in flag. For example:

Deprecating useTransition in favor of useNavigation - this was a breaking change but they can opt-into the new behavior by using the new hook. So we didn't need a future flag and instead can just log a deprecation warning when they call useTransition.

Removing fetcher.type/fetcher.submission - this was a breaking change but they can opt-into the new behavior by changing their code to look at fetcher.state instead. So again no future flag needed and instead can just log a deprecation warning when they access fetcher.type/fetcher.submission.

v1 -> v2 route conventions - There's no way they can opt into this in the routes/ directory so we needed to provide a future flag.

Normalizing fetcher.formMethod from post -> POST - there was no way they could do this at the call site using existing APIs so we had to give them a global future flag.

So that diagram needs a new path for breaking changes where we say "local opt-in possible?" and if so we can just log deprecation warnings and don't need to introduce a future flag.

docs/hooks/use-submit.md

packages/react-router-dom/dom.ts

Co-authored-by: Mark Dalgleish <[email protected]>

brophdawg11 · 2023-04-18T18:28:04Z

packages/router/router.ts

@@ -147,7 +147,7 @@ export interface Router {
    key: string,
    routeId: string,
    href: string | null,
-    opts?: RouterNavigateOptions
+    opts?: RouterFetchOptions


This was an underlying type bug, but unnoticed since they're effectively identical. The changes below properly remove both replace/state from fetch() options

docs/hooks/use-fetcher.md

docs/hooks/use-navigation.md

markdalgleish · 2023-04-19T05:55:29Z

docs/hooks/use-submit.md

+
+```tsx
+let obj = { key: "value" };
+submit(obj); // -> request.formData()


Should there be additional type-checking for this use case (and I guess when explicitly setting encType: 'application/x-www-form-urlencoded')?

If I understand correctly, it seems like a big footgun that I can now pass any type here and I won't get a type error and then it'll blow up later when the code assumes it can be serialized to FormData.

Probably 😬 . #10362 starts adding some more advanced type checks around payload/encType/action so we can try to add it there - but it's proven a bit tricky thus far to get the inferred types working right across params. I'm sure we can figure something out though. I'll drop a note over there

Co-authored-by: Mark Dalgleish <[email protected]>

brophdawg11 · 2023-04-21T18:45:27Z

packages/react-router-dom/dom.ts

+        // When a raw object is sent - even though we encode it into formData,
+        // we still expose it as payload so it aligns with the behavior if
+        // encType were application/json or text/plain
+        payload = target;


@ryanflorence I went slightly against the mutual exclusivity here so that application/x-www-form-urlencoded would better mimic the behavior of application/json and text/plain.

formData and payload remain mutually exclusive when submitting an HTMLElement, FormData, or URLSearchParams instance. But not when you submit a raw object.

This reverts commit 9d81bf6.

github-actions · 2023-04-26T16:48:20Z

🤖 Hello there,

We just published version 6.11.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

brophdawg11 · 2023-04-26T17:19:08Z

Silly bot, these were reverted from dev so they're not in 6.11.0-pre.0. We plan to get them re-added with the latest changes from the Proposal in 6.12.

)"" This reverts commit f92aa2e.

remix-cla-bot bot added the CLA Signed label Apr 13, 2023

brophdawg11 self-assigned this Apr 13, 2023

Base automatically changed from brophdawg11/stable-navigate to dev April 14, 2023 19:08

brophdawg11 added 5 commits April 14, 2023 15:19

Opt-out of submission serialization via encType:null

f963974

Add support for application/json and txt/plain serialization

66aa3ab

Add changesets

d424514

Bundle Bump

b01d57e

Few more tests

c945785

brophdawg11 force-pushed the brophdawg11/opt-out-serialization branch from 6608193 to c945785 Compare April 14, 2023 19:19

brophdawg11 added 3 commits April 14, 2023 15:51

Docs

67a7444

Add fetcher tests

ef95de9

Update serialization logic

4b77992

brophdawg11 commented Apr 14, 2023

View reviewed changes

Revert serialization change

7e7768e

brophdawg11 commented Apr 14, 2023

View reviewed changes

brophdawg11 marked this pull request as ready for review April 14, 2023 20:50

brophdawg11 mentioned this pull request Apr 17, 2023

Support direct handlers in useSubmit/fetcher.submit/fetcher.load #10362

Merged

markdalgleish reviewed Apr 17, 2023

View reviewed changes

docs/hooks/use-submit.md Outdated Show resolved Hide resolved

markdalgleish reviewed Apr 17, 2023

View reviewed changes

docs/hooks/use-submit.md Outdated Show resolved Hide resolved

markdalgleish reviewed Apr 17, 2023

View reviewed changes

packages/react-router-dom/dom.ts Outdated Show resolved Hide resolved

MichaelDeBoey added docs react-router-dom router labels Apr 17, 2023

brophdawg11 and others added 4 commits April 18, 2023 09:50

Apply suggestions from code review

01656c7

Co-authored-by: Mark Dalgleish <[email protected]>

Clarify comment

42c72aa

Move payload to ActionFunctionArgs

13e6b8e

Clean up router navigate/fetch argument types

8760827

brophdawg11 added 2 commits April 18, 2023 14:12

Update docs

91f97ba

Remove unintentional action type

97425bc

brophdawg11 commented Apr 18, 2023

View reviewed changes

markdalgleish reviewed Apr 19, 2023

View reviewed changes

docs/hooks/use-fetcher.md Outdated Show resolved Hide resolved

markdalgleish reviewed Apr 19, 2023

View reviewed changes

docs/hooks/use-navigation.md Outdated Show resolved Hide resolved

markdalgleish reviewed Apr 19, 2023

View reviewed changes

Apply suggestions from code review

046bca4

Co-authored-by: Mark Dalgleish <[email protected]>

markdalgleish approved these changes Apr 19, 2023

View reviewed changes

brophdawg11 added 3 commits April 20, 2023 10:12

Merge branch 'dev' into brophdawg11/opt-out-serialization

e1c71cb

Remove hard requirement of mutual exclusivity

99c9a0e

Add docs to route.action

5cdbdab

brophdawg11 commented Apr 21, 2023

View reviewed changes

Fix lint issue

d8dc40a

brophdawg11 merged commit 9d81bf6 into dev Apr 21, 2023

brophdawg11 deleted the brophdawg11/opt-out-serialization branch April 21, 2023 18:54

brophdawg11 added a commit that referenced this pull request Apr 26, 2023

Revert "Add better control over submission serialization (#10342)"

f92aa2e

This reverts commit 9d81bf6.

brophdawg11 added a commit that referenced this pull request Apr 28, 2023

Revert "Revert "Add better control over submission serialization (#10342

4d58d0e

)"" This reverts commit f92aa2e.

remix-run deleted a comment from github-actions bot Apr 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add better control over submission serialization #10342

Add better control over submission serialization #10342

brophdawg11 commented Apr 13, 2023 •

edited

Loading

changeset-bot bot commented Apr 13, 2023 •

edited

Loading

brophdawg11 Apr 14, 2023 •

edited

Loading

brophdawg11 Apr 21, 2023

brophdawg11 Apr 14, 2023 •

edited

Loading

brophdawg11 Apr 14, 2023

brophdawg11 Apr 21, 2023

markdalgleish Apr 17, 2023

brophdawg11 Apr 18, 2023 •

edited

Loading

brophdawg11 Apr 18, 2023

markdalgleish Apr 19, 2023

brophdawg11 Apr 19, 2023

brophdawg11 Apr 21, 2023

github-actions bot commented Apr 26, 2023

brophdawg11 commented Apr 26, 2023

		\| { formData: FormData; payload?: undefined }
		\| { formData?: undefined; payload: any }

Add better control over submission serialization #10342

Add better control over submission serialization #10342

Conversation

brophdawg11 commented Apr 13, 2023 • edited Loading

changeset-bot bot commented Apr 13, 2023 • edited Loading

🦋 Changeset detected

brophdawg11 Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brophdawg11 Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brophdawg11 Apr 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 26, 2023

brophdawg11 commented Apr 26, 2023

brophdawg11 commented Apr 13, 2023 •

edited

Loading

changeset-bot bot commented Apr 13, 2023 •

edited

Loading

brophdawg11 Apr 14, 2023 •

edited

Loading

brophdawg11 Apr 14, 2023 •

edited

Loading

brophdawg11 Apr 18, 2023 •

edited

Loading