feat: warn if page endpoint body is not serializable #5635
Conversation
🦋 Changeset detectedLatest commit: 6780d98 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 |
| import { coalesce_to_error } from '../../../utils/error.js'; | ||
| import { domain_matches, path_matches } from './cookie.js'; | ||
| import { is_pojo, warn_if_not_serdeable } from './utils.js'; |
There was a problem hiding this comment.
TIL that serdeable is an actual word 😄
There was a problem hiding this comment.
Serde is common in the rust circles. Guess it hasn't carried over much outside, will change if it's not clear although need suggestions cause I'm horrible at it.
| /** | ||
| * @param {any} v | ||
| */ | ||
| export function is_pojo(v) { |
There was a problem hiding this comment.
It irks me a little that there's another function with the same name which does something different, and both have no short description so you have to jump into the code and usage locations to know what's this about.
Also: What's the TODO about?
There was a problem hiding this comment.
Yeah, the other one does a lot of other stuff specific to Response body types that's not actually checking if it's a plain object. I did want to rename it but likely in a separate PR that touches that part of the code.
The TODO is lodash's implementations making me paranoid about some weird JavaScript behaviors that I'm unaware of.
There was a problem hiding this comment.
Should we add a comment about this with a link to lodash's implementation and something like "if people ever run into weird edge cases where this doesn't behave as expected, look into lodash's implementation more closely"?
|
|
||
| /** @param {any} v */ | ||
| export function is_serializable_primitive(v) { | ||
| // TODO: javascript-isms |
|
Gmail told me to look at the CI, and jesus christ that's scary, going to look at it tomorrow |
|
Ughh, |
|
It occurred to me that since we always need to serialize the data, we can do the checks at serialization time with a custom |
#4944 #5358 #5545
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. All changesets should bepatchuntil SvelteKit 1.0