-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: explain why json()
cannot be used in form actions
#12829
Conversation
🦋 Changeset detectedLatest commit: b8e8cc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Co-authored-by: Ben McCann <[email protected]>
Co-authored-by: Ben McCann <[email protected]>
@@ -280,6 +280,14 @@ function try_deserialize(data, fn, route_id) { | |||
// If we're here, the data could not be serialized with devalue | |||
const error = /** @type {any} */ (e); | |||
|
|||
// if someone tries to use `json()` in their action | |||
if (data instanceof Response) { |
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.
there's nothing really specific in this error message about a Response
, so I'm wondering why the if
below doesn't catch it. It seems like we could combine the two. Maybe they need to check if it's an instance of DevalueError or if name is DevalueError?
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.
It still catches it but the error message is usually "Cannot stringify arbitrary non-POJOs" which can be unhelpful. We could combine the two but I'm wondering if there are instances where devalue's error message is preferred.
closes #12787 Adds a check to see if the data being returned is a Response object (such as with the helper method json()), then replies with a more helpful error message. --------- Co-authored-by: Ben McCann <[email protected]>
closes #12787
Adds a check to see if the data being returned is a
Response
object (such as with the helper methodjson()
), then replies with a more helpful error message.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits