-
-
Notifications
You must be signed in to change notification settings - Fork 2k
check if data is serializable #5987
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
Conversation
🦋 Changeset detectedLatest commit: 69cbd4b 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 |
const tag = Object.prototype.toString.call(value); | ||
if (tag === '[object Object]') { |
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.
This wouldn't work for custom classes. If they pass through they'll fail instanceof
checks and their methods aren't iterated over by either Object.keys
or for..in
.
const tag = Object.prototype.toString.call(value); | ||
if (tag === '[object Object]') { |
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.
luckily i know where i can find a more robust check!
const tag = Object.prototype.toString.call(value); | |
if (tag === '[object Object]') { | |
// This simple check might potentially run into some weird edge cases | |
// Refer to https://github.com/lodash/lodash/blob/2da024c3b4f9947a48517639de7560457cd4ec6c/isPlainObject.js?rgh-link-date=2022-07-20T12%3A48%3A07Z#L30 | |
// if that ever happens | |
if (Object.getPrototypeOf(value) === Object.prototype) { |
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.
That one is essentially "if it's not a freshly constructed plain object then no bueno",
I think it'll miss Object.defineProperties and other stuff, but if people are going to those lengths for the returned object, I wouldn't want to accomodate that.
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.
Also, are we going to need these checks if #6008 is implemented?
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.
No, in that case we'd have to rely on devalue
throwing an error if stuff isn't serializable. Though it would be preferable to serialize the entire array in one go, which would make it tricker to track down the error... I haven't really thought about the best way to do that yet, but in any case #6008 probably won't happen for a while (feels like a post-1.0 thing)
ObjectIds get blocked despite passing a |
This throws an error if a
load
function in+page.server.js
or+layout.server.js
returns data that can't be serialized as JSON, so things like/hello/
don't become{}
.Supersedes #5635.
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. All changesets should bepatch
until SvelteKit 1.0