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

Make it possible to return more than JSON from server-only load functions #6008

Closed
Rich-Harris opened this issue Aug 18, 2022 · 48 comments · Fixed by #6318
Closed

Make it possible to return more than JSON from server-only load functions #6008

Rich-Harris opened this issue Aug 18, 2022 · 48 comments · Fixed by #6318
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.

Comments

@Rich-Harris
Copy link
Member

Describe the problem

Right now, the output of load in +layout.server.js or +page.server.js has to be serialized with JSON.stringify so that it can be transported to the client.

Occasionally it's useful to transport values that can't be serialized as JSON. Date springs to mind, along with things like this, where anything expecting selected to be a member of options post-deserialization will be disappointed:

export function load() {
  const options = [
    { flavour: 'chocolate' },
    { flavour: 'strawberry' },
    { flavour: 'banana' }
  ];

  const selected = options[0];

  return { options, selected };
}

Describe the proposed solution

We have a proven technique for this: https://github.com/rich-harris/devalue. It's fast, and doesn't involve a deserialization step. (It doesn't currently support BigInt though, we'd need to add that.)

To work with CSP, we can't eval the result (including by creating a <script>), we need to import it. Since we need to get fresh data when navigating back to a previously-visited page, we need to use a cachebusting mechanism; since modules live in a cache that we have no way of purging, we need to be careful to avoid memory leaks.

We can't therefore do this...

export const data = {...};

...and must instead do this, so that the data can be garbage collected once it's been used:

window.__data = {...};

On the client side, it would look something like this:

let uid = 1;

// later
await import(`${url.pathname}/__data.js`);
const data = window.__data;
delete window.__data;

Alternatives considered

There's a variety of alternatives to devalue listed here: https://github.com/rich-harris/devalue#see-also, though I'm not aware of any reasons to favour them.

Or, we could just stick to JSON, which has better performance characteristics if you have a huge amount of data.

Importance

nice to have

Additional Information

No response

@Rich-Harris Rich-Harris added the feature / enhancement New feature or request label Aug 18, 2022
@Conduitry
Copy link
Member

Is it sensible to make it a build time configuration concern whether to use JSON or devalue for a given page? (We obviously don't want to be doing this analysis at runtime and the client would have no way of knowing the result of this analysis ahead of time.) Explicitly opting in to the more performant JSON might be nice.

@benmccann benmccann added the p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc. label Aug 18, 2022
janosh added a commit to janosh/awesome-sveltekit that referenced this issue Aug 22, 2022
…g]/+page.server.ts

fixes error reported in sveltejs/kit#6093
might be fixed upstream in sveltejs/kit#6008
@ghost

This comment was marked as off-topic.

@ghost
Copy link

ghost commented Aug 24, 2022

My last comment was on-topic. devalue doesn't support ObjectIds and they get blocked despite passing a JSON.stringify({ a: new ObjectId() }). The deserialization would need an new ObjectId(...) but even without (just being a string) it'd be still fully reversible.

@chanced
Copy link

chanced commented Aug 24, 2022

Currently, check_serializability doesn't check to see if an object has a toJSON method. This would solve serialization of Date and @2foo's ObjectId issue.

What I'm not sure of is if there's a reason for that. Is this due to concerns over deserialization or typings?

@Conduitry
Copy link
Member

We're currently specifically not checking for .toJSON(), because things serialized that way would deserialize (JSON.parse) differently in the browser. You would have different data in the component depending on whether it was rendered on the server or in the browser. See e.g. #6093.

@chanced
Copy link

chanced commented Aug 24, 2022

That makes sense. While a bit of a performance hit, would it not make sense to run serverside through a serde pass-through?

If so, the need for check_serializability would presumably be diminished.

edit: I guess this could cause an issue with typings.

@ghost
Copy link

ghost commented Aug 24, 2022

or (again) why can't we specify out own serializer, e.g. superjson which supports all extra types and recipes for stuff like Prisma.Decimal + ObjectId and call it a day? why do you want to reinvent the wheel?

image

@Conduitry
Copy link
Member

The current plan is to use devalue, a library that already exists, and which superjson's readme cites as prior art, and which has better performance characteristics than superjson.

@ghost
Copy link

ghost commented Aug 24, 2022

@Conduitry thanks for your immediate feedback.

devalue, a library that already exists, and which superjson's readme cites as prior art,

This is really nice and congrats to Rich's work on devalue but both reasons do not address the interests of your users and shouldn't be considered.

which has better performance characteristics than superjson.

How can devalue have better perf if it doesn't support all the types nor recipes as superjson does?

My feeling is that you hesitate to integrate a lib from a competing stack (Blitz) for whatever reason. This is ok and I do not care for now.

But, I'd welcome some outlook and more that we address this post 1.0.0 ETA. The users need to migrate to the latest next version now because the longer they wait the harder the migration will get bc of the major routing overhaul.

This silent change and intro of check_serializability is not jut a breaking change, it's blocking.

Hence, I'd appreciate if you guys rethink you decision for/against superjson and/or commit to some ETA for the integration of devalue in SvelteKit + integration of all lacking types.

Would be also great if @Rich-Harris could give his take, I mean what is your recommendation now for users like us, wait and hope?

@Beiri22
Copy link

Beiri22 commented Aug 24, 2022

As you know from my other issue, i'd also like to see the possibility to customize serialization/deserialization to accommodate for custom user needs. (like cbor, superjson, devalue, ...) Seems like an easy abstraction to apply.

@ifiokjr
Copy link

ifiokjr commented Aug 24, 2022

This sounds helpful.

I've just bumped into the situation where serializing the Date type throws an error.

A potential fix could be borrowed from trpc where end-users can set a transformer property to support custom serialization over the network.

https://github.com/trpc/trpc/blob/a321eac66d4fed9fec4ad94d2a0336353b35aebc/www/docs/server/data-transformers.md?plain=1#L43

I'd like to see something similar in SvelteKit for page endpoints.

This could be added to the hooks.js file.

// hooks.js
import superjson from 'superjson';

// This hook adds custom serialization of json within page endpoints.
export const pageDataTransformer = superjson;

The pageDataTransformer has serialize and deserialize methods.

When the hook is set, SvelteKit would automatically serialize page endpoint data being sent from the server to the client, and deserialize it on the client side.

The advantage over providing a blessed implementation like devalue is that alternatives like superjson allow users to extend the serializable types.

My own use case involves passing Uint8Array binary data between the server and client. This would require a change to the devalue codebase, but can be added to superjson trivially.

If this is useful, I can create a pull request.

@alexturpin
Copy link

+1 for an approach that allows plugging a serializer like superjson. I already use it with trpc to serialize Temporal date objects and it's a dream.

@ghost
Copy link

ghost commented Aug 25, 2022

we address this post 1.0.0 ETA

@Rich-Harris why do you want wait so long? Users face breaking/blocking changes now and shall wait for some months? Because devalue isn't ready? @ifiokjr offered already a PR/help, for what are you waiting? Looking fwd to your feedback!

@Rich-Harris
Copy link
Member Author

Alright, since I'm getting called out via childish memes let me articulate why we are planning to use devalue.

These are our options:

  • continue using JSON, which everyone seemed perfectly happy with before I opened this issue. This is the most performant approach, and it requires no deserialization logic in your app, but it means that people can't use Date etc by default
  • use devalue, which is slower to serialize data than JSON.stringify but still plenty fast, and allows us to use Date, repeated/cyclical references and so on. Because it generates JavaScript, there's no need for deserialization logic, meaning it's extremely fast on the client and doesn't bloat your app
  • use superjson by default. superjson is significantly slower at serialization than devalue or JSON.stringify, and the serialized data weighs significantly more than the devalue equivalent. Furthermore, you need to add the deserializer to your app, which is 12kb (before custom deserialization logic) — an unacceptable amount to include by default
  • allow custom (de)serialization. This would make some of the people in this thread happy, but would mean that for the default case you wouldn't be able to use Date or repeated/cyclical references, so in order to use those you would basically need to use superjson (with all the associated performance/size penalties) or some hand-rolled function (watch out for those XSS vulnerabilities!). It's also extra complexity/API surface area. Because of how devalue works (i.e. it doesn't transport JSON) it wouldn't be an option as a custom (de)serializer

The one limitation of devalue is that it doesn't support custom classes like ObjectId or Prisma.Decimal. But I'm genuinely curious (as a user of neither Mongo nor Prisma) why this is necessary. What can you do with those values on the browser that wouldn't involve re-serializing them to send them back to the server (or using a client-side SDK in the first place)? If it's such a crucial requirement, how are you using SvelteKit today?

Bear in mind that you already have the ability to (de)serialize data however you want by returning JSON-able data from load in +page.server.js then processing it in the load in +page.js. It might not be as idiomatic as app-wide custom (de)serialization, but it gives you complete flexibility at zero cost to the users of the framework that don't share your needs, and can easily be abstracted into a helper function.

Given all these trade-offs, devalue seems like the obvious choice.

@Rich-Harris
Copy link
Member Author

@2foo

why do you want wait so long?

Where did I say anything about delaying?

Because devalue isn't ready?

People have been using devalue in production since before SvelteKit existed.

Not sure what you hope to achieve by sending a spammy GitHub notification to my boss, but maybe rethink that?

@Beiri22
Copy link

Beiri22 commented Aug 25, 2022

Because of how devalue works (i.e. it doesn't transport JSON) it wouldn't be an option as a custom (de)serializer

Does it need to emit JSON in order to be a serializer? I thought an arbitrary string / byte array could be the result of the serialization, so devalue would in my opinion fit as a default choice when allowing a custom serializer. You get:

  • serialize(any): string | byte array
  • deserialize(string | byte array): any
  • mime type:string

@Rich-Harris
Copy link
Member Author

Since devalue generates JavaScript code, the way to 'deserialize' it is to eval it. But that's not a workable solution, because many sites have CSP rules that forbid eval. The solution presented here is CSP-compliant because code you import inherits CSP exemptions.

As for byte arrays: yes, if you can take the whole data object returned from the server-only load and turn it into a binary format then you could apply the reverse transformation to response.arrayBuffer(), but only for data fetched during client-side navigation. For the serialized data that's included in the initial HTML response you would need to convert it to a string, most likely using base64 which makes the data 33% larger (and less compressible). It's possible that we could get it to work, but it's unlikely to be an optimal solution. At that point you'd probably be better off loading data from a +server.js function where you retain complete control over the shape of the response.

@ifiokjr
Copy link

ifiokjr commented Aug 25, 2022

@Rich-Harris, to make it clear, I love what you're doing with svelte. As a maintainer of much smaller open-source projects, I'm amazed at how well you've been able to communicate and implement such an ambitious vision.

Regarding the childish memes, I've found it's always the people who produce nothing of value that resort to such pettiness.

Bear in mind that you already have the ability to (de)serialize data however you want by returning JSON-able data from load in +page.server.js then processing it in the load in +page.js. It might not be as idiomatic as app-wide custom (de)serialization, but it gives you complete flexibility at zero cost to the users of the framework that don't share your needs, and can easily be abstracted into a helper function.

I was not aware of this. I'll try it out. The new routing API means I'm still getting to grips with what's possible.

@ghost
Copy link

ghost commented Aug 25, 2022

The one limitation of devalue is that it doesn't support custom classes like ObjectId or Prisma.Decimal. But I'm genuinely curious (as a user of neither Mongo nor Prisma) why this is necessary.

Since you intro'd the serializability checker, responses containing ObjectIds don't go through anymore. All SSR endpoints including ObjectIds broke, every single one.

Now, we would need to filter out or transform ObjectId values on app or Mongo level before sending them. This creates unnecessary run(s) and perf hits.

I wouldn't even need to have ObjectId deserialized back to ObejctIds, they work as string well (if you always wrap them in a new ObjectId()). Just allow ObjectId's in the checker.

If it's such a crucial requirement, how are you using SvelteKit today?

Few days ago before the checker was introduced, everything was working fine. I've the feeling you are not even aware that you created a blocking change.

Bear in mind that you already have the ability to (de)serialize data however you want by returning JSON-able data from load in +page.server.js then processing it in the load in +page.js

Sounds like a bigger hassle and wouldn't I lose types?

At that point you'd probably be better off loading data from a +server.js function where you retain complete control over the shape of the response.

Then we'd lose SSR and what's the point of SvelteKit anymore?

Where did I say anything about delaying?

In your og post, I quoted this already: "we address this post 1.0.0 ETA"

allow custom (de)serialization. This would make some of the people in this thread happy, but would mean that for the default case you wouldn't be able to use Date or repeated/cyclical references,

Aren't cyclical references an anti-pattern anyway?

To move forward: Just make devalue the default but allow custom serializers.

And latter pls ASAP or remove the checker or allow ObjectIds in the checker.

@chanced
Copy link

chanced commented Aug 25, 2022

Considering devalue will serialize functions, are you handing folks a footgun with this?

eval(`(${devalue_serialized})`)

Sorry the question is offtopic. The reason I'm asking is because that right now, folks who use devalue had to go looking for it and thus (hopefully) have some inclination as to how to implement it. Even if that knowledge comes at the expense of debugging.

I'm not sure that will be the case if its baked into a framework.

@ifiokjr
Copy link

ifiokjr commented Aug 25, 2022

Bear in mind that you already have the ability to (de)serialize data however you want by returning JSON-able data from load in +page.server.js then processing it in the load in +page.js. It might not be as idiomatic as app-wide custom (de)serialization, but it gives you complete flexibility at zero cost to the users of the framework that don't share your needs, and can easily be abstracted into a helper function.

I've just added this to my project, and it works perfectly! 🎉

For those who bump into this issue. Here's some sample code.

  1. First load the data in +page.server.ts and serialize it with superjson
// src/routes/[name]/+page.server.ts
import superjson from 'superjson';
import fetchSomething from '$lib/fetch-something';
import type { PageServerLoadEvent } from './$types.js';

export async function load(event: PageServerLoadEvent) {
  const { name } = event.params;
  const something = await fetchSomething(name);

  // Serialize the data that is returned.
  return superjson.serialize(something);
}
  1. Now deserialize the data in the +page.ts and return the value.
// src/routes/[name]/+page.ts
import superjson, { type SomethingType } from 'superjson';
import type { PageLoadEvent } from './$types.js';

export async function load(event: PageLoadEvent) {
  // Deserialize the data provided
  const something: SomethingType = superjson.deserialize(event.data);

  // Return the deserialized data.
  return { something }
}
  1. Finally, consume the data in your route component.
<script lang="ts">
  // src/routes/[name]/+page.svelte
  import type { PageData } from './$types.js';

  export let data: PageData;
  const decoder = new TextDecoder('utf8');

  $: base64Image = btoa(decoder.decode(data.something.image));
</script>

<h1>{data.something.name}</h1>
<p>{data.something.description}</p>
<img src="data:image/jpeg;charset=utf-8;base64, ${base64Image}" alt="{data.something.alt}" />

Knowing that this is possible, I completely understand your desire to use devalue since it provides minimal cost to the majority of users (1.9kb) and there's an escape hatch for routes that need more functionality.

@ghost
Copy link

ghost commented Aug 25, 2022

@ifiokjr you created additional runs (=perf hits), code gets boilerplate-y/bloated and not sure if you keep TS types (can you check pls?). idk if that's the solution we're looking for...

@chanced
Copy link

chanced commented Aug 25, 2022

@2foo dude, he gave you the code. Check for yourself.

@chanced
Copy link

chanced commented Aug 25, 2022

No you do not.

@ghost
Copy link

ghost commented Aug 25, 2022

wdym?

@Rich-Harris
Copy link
Member Author

@2foo Ugh. Here we go, one by one:

responses containing ObjectIds don't go through anymore. All SSR endpoints including ObjectIds broke, every single one

Prior to this change, people were returning Date objects that were being silently converted to strings, resulting in broken apps and incorrect generated types. That's a far worse form of breakage.

Sounds like a bigger hassle and wouldn't I lose types?

It's a hassle if you insist on trying to return non-serializable data. As I said, you can create a helper function that does it for you.

No, you wouldn't lose types, unless you're using superjson incorrectly.

In your og post, I quoted this already: "we address this post 1.0.0 ETA"

The only person on this thread who said anything along those lines is you. What the hell are you talking about? (Actually, don't respond — I genuinely don't care.)

And latter pls ASAP or remove the checker or allow ObjectIds in the checker.

No. But if you're going to continue to behave like this — making demands and (I'm guessing from contextual evidence that it was you) making memes deriding the makers of the open source software you use — then I would prefer that you go and use a different framework. The community doesn't need your negative energy, and I personally have better things to do than waste time engaging with you.

you created additional runs (=perf hits)

It's the same steps that SvelteKit would be doing on your behalf 🤦

@ifiokjr thanks, this is an awesome example! Note that devalue's 1.9kb is only added to your server-side code — on the client-side it's completely free (because it's just JavaScript)

@chanced

Considering devalue will serialize functions

It won't — it'll fail if you hand it a non-serializable object. I just threw this quick REPL together so you can get a feel for how it works: https://svelte.dev/repl/138d70def7a748ce9eda736ef1c71239?version=3.49.0

@chanced
Copy link

chanced commented Aug 25, 2022

@Rich-Harris

You're awesome. Keep up the good work.

@chanced
Copy link

chanced commented Aug 25, 2022

I should have read the devalue readme more closely. 🤦‍♂️

I saw this:

devalue(obj); // '(function(a){a.a=1;a.b=2;a.c=3;a.self=a;return a}({}))'

and assumed it was serializing functions. What I didn't read was

If devalue encounters a function or a non-POJO, it will throw an error.

Anyway, thank you for taking the time to put together the example despite my egregious RTFM fail on my part.

@0gust1
Copy link
Contributor

0gust1 commented Aug 25, 2022

I was wondering if Error (and eventually subclasses of it) could be serializable ?

context:
In our current app we have a small semantic error system (e.g. DatabaseError, NetworkError, ExternalHTTPError, FSError)
It's quite useful in our service layer and in endpoints to build the appropriate responses, construct meaningful error messages and accurate logs. (we're also using the 'neverthrow' pattern : our service/business functions always return a DataOrError union type).
We used a OO system (meh) because we wanted to add extra fields on some errors (like the original error message of an external HTTP call made server-side).

And then, few months ago, very naively, we attempted to use those semantic errors on the browser side (if(e instanceOf MyErrorType){...}), and we discovered that Error doesn't serialize well 😄 .

edit: I realize it may be more a devalue/whatever question than Sveltekit one, if so, apologies.

@Rich-Harris
Copy link
Member Author

Errors are serialized as POJOs using this logic:

export function error_to_pojo(error, get_stack) {
if (error instanceof HttpError) {
return /** @type {import('./page/types').SerializedHttpError} */ ({
message: error.message,
status: error.status,
__is_http_error: true // TODO we should probably make this unnecessary
});
}
const {
name,
message,
stack,
// @ts-expect-error i guess typescript doesn't know about error.cause yet
cause,
...custom
} = error;
/** @type {Record<string, any>} */
const object = { name, message, stack: get_stack(error) };
if (cause) object.cause = error_to_pojo(cause, get_stack);
for (const key in custom) {
// @ts-expect-error
object[key] = custom[key];
}
return object;
}

As such, the deserialized objects won't pass any instanceof checks, but you can check for things like this:

if ($page.error.name === 'DatabaseError') {

@mostrecent
Copy link

mostrecent commented Aug 26, 2022

What can you do with those values on the browser that wouldn't involve re-serializing them to send them back to the server

Mongo user here: You use stringified ObjectIds for URL params in the client without the need to deserialize them back, e.g. you get a list of bikes and want to link each entry to their detail pages:

// Mongo sends this `result`:
[ { _id: new ObjectId("00000020f51bb4362eee2a4d"), name: 'fast bike'}, 
  { _id: new ObjectId("00000020f51bb4362eee2a5d"), name: 'turbo bike'} ]

// JSON.stringify(result) creates:
'[{"_id":"00000020f51bb4362eee2a4d","name":"fast bike"},{"_id":"00000020f51bb4362eee2a5d","name":"turbo bike"}]'

// in .svelte you can use both results with the same code
{#each bikes as bike}
  <a href="/bikes/{bike._id.toString()}" />
{/each}

So, you actually never deserialize _id in the client. Not sure if I answered your question but yeah.

All solutions for dealing with ObjectIds I can think of are subpar (but maybe there's a better way):

  • You could just JSON.parse(JSON.stringify(result)) before in order to keep SSR still working and let the JSON pass but this leaves Dates as string as well
  • Using superjson with the drawbacks you've mentioned already
  • Using devalue but we would still need to do a JSON.parse(JSON.stringify(result)) before; this would leave Dates as string which devalue or (0,eval)(...) cannot deserialize anymore

It's actually a tricky situation for SEO sites pulling tons of data from Mongo and need to be server-side rendered.

@dummdidumm
Copy link
Member

I know Rich likely doesn't agree, but part of the solution in my opinion is to make the error a warning instead - you are warned that this doesn't deserialize correctly, but if you know what you are doing, you should be allowed to.

@mostrecent
Copy link

@dummdidumm something like this? https://gist.github.com/mostrecent/ac06952b1c204d72436eb868faa057c3#file-warn_if_object_id-js-L26-L29

@chanced
Copy link

chanced commented Aug 26, 2022

@mostrecent I imagine warn if the check_serializability fails. Adding the bson package for everybody isn't cordial.

@mostrecent
Copy link

mostrecent commented Aug 26, 2022

@chanced right, while bson-objectid would run on the server only and is 1.3kB gzipped (a streamlined bson package), we can also rip their duck-typing test which is just a few lines and remove the dependency entirely: https://gist.github.com/mostrecent/3cdcc36938c29a1d25defc6f381483c4#file-warn_if_object_id2-js-L26-L31 (we'd still need the console.warn() as in the first gist)
Regarding your first point I'd agree as well.

@Rich-Harris
Copy link
Member Author

ObjectId isn't special, it's just one example of a more general phenomenon — there's no way we're going to check for it specifically.

Kinda feels like the best thing to do (if you don't need to reconstruct the ObjectId instance in the client, in which case you'd want superjson or whatever) would be to walk the tree of mongo data and do this before returning it:

if (node._id) node._id = node._id.toString();

That logic could presumably live in a small wrapper around the mongo client. It's a very small amount of extra work but it prevents all the type safety issues etc that you'd inevitably run into by relying on the fact that thing._id.toString() happens to work before or after serialization.

(Seems like a better solution still would be for mongo to return serializable data in the first place, but like I say, I'm not a mongo user...)

@chanced
Copy link

chanced commented Aug 26, 2022

There could be ObjectIds in fields/values other than _id. You'd basically have to check the type then walk the obj/array.

In both go and rust, you can serde an ObjectId into a string with custom (un)marshaling logic. After a cursory search, I'm not sure that's possible with the node driver without forking the bson package.

I guess using something like superjson would make sense for people in this predicament. There really isn't a need to deserialize into a uuid/objectid though.

@mostrecent
Copy link

mostrecent commented Aug 26, 2022

Kinda feels like the best [...] to walk the tree of mongo

@Rich-Harris definitely an option and I've been exploring various solutions since you posted this hint, from doing that in Mongo with $toString which is surprisingly slow (a 2ms request gets 20ms) or keeping it simple by just stringifiying only ._id which is fast but you miss foreign keys as @chanced mentioned, which is again not that important on the client but not great either, it just doesn't feel right. I haven't found the best way yet but will keep trying over the weekend.

For now, I'd like to put myself into your/Rich's shoes. You can imagine that I'm biased (we heavily use Mongo on a very large scale) but let's give it a try (this is not about Mongo vs other DBs!):

First, I assume that Rich's (and Vercel's) goals are following—please correct me if I'm wrong:

  • Best performance, lowest footprint and consistent design choices
  • Adoption and good organic growth, not too fast but not too slow either
  • Great onboarding, either via education, e.g. the gold-standard tutorials or again via consistent and/or intuitive design choices which again helps adoption and growth
  • (only Vercel): putting not all eggs in one basket (Next), and having a huge user bases across the board in order to have a bigger/safer conversion funnel to Vercel's services, but these goals should be neglected for now

Now, let's look at MongoDB. Mongo doesn't have the best reputation but it still shines in a few areas (I make it short just to give some background): It's pretty easy to scale writes as replica sets (multi-master), you can run it everywhere keeping costs low + flexibility high without being locked into one vendor (self-host, AWS, Google Cloud, Azure) and schemas, e.g. json-schemas, can bring you type-safety-wise quite far if you know what you're doing. Mongo has for sure drawbacks and please, let's not discuss what's the best DB is, it's more to give context to following data.

How many users are using Mongo now? How many of them are potential SvelteKit users? Looking at this thread, not so many. But maybe this is because of the (currently) smaller user base of SvelteKit. If we look at npmtrends[1], we see that's it's not just popular but outperforms many others, indicated by driver downloads. This ignores users who write raw SQL queries or those from Dynamo and Firebase but should give a notion of how many users use Mongo. This could be also legacy users but we would need to dig deeper into this to have full clarity. At least we know, it's definitely not a small user base.

Back to SvelteKit: This current ObjectId issue is definitely solvable and people who are already invested will invest further to solve this problem somehow. The question is though, will new users see over this ObjectId issue and invest the same time and energy to make ObjectIds work? The solution is not that straight-forward as we have seen in this thread and could create enough headache to let users give up (because they aren't invested yet). Or would they just switch to a "safer" choice? Because of SEO? We know SSR is still highly crucial, if not the key requirement for non-app sites.

The trade-off is:

|----------------------------------------------------------------------------|
Consistent design choices                               Please a huge chunk of 
                                                                   Mongo users

Most of us will have a quick and good answer ready in their mind. But every decision will come with a sacrifice and we should be aware of its impact and maybe sleep a night over it.

Footnotes
[1]:
npmtrends
image

The next chart is from db-engines which has a specific ranking algorithm which can be questioned. I wanted just to add some non-scientific market share data quickly to prove that there's a huge user base and not that it's a better DB than others:
image

Docker Hub download figure:
image

@Rich-Harris
Copy link
Member Author

Again, we're not going to start shoving hacks in the framework just because some people use mongo. All that would happen is people would start wondering why their ObjectId classes are mysteriously being stringified. If it's that important, someone will create a sanitize-mongo-data package. Not a big deal.

@benmccann benmccann linked a pull request Aug 26, 2022 that will close this issue
7 tasks
dummdidumm pushed a commit that referenced this issue Aug 29, 2022
…6318)

closes #6008
fixes #6357

This allows load functions to return things like Date objects, regexes, Map and Set, BigInt and so on. It also allows repeated and cyclical references, for the times when they're useful.
@mostrecent
Copy link

mostrecent commented Aug 30, 2022

someone will create a sanitize-mongo-data package. Not a big deal.

I agree, here's the gist. I could get it fast but it messes up TS types: When using this as standard wrapper all ObjectId props need to be typed as string. Then, you can't save ObjectId props as such anymore, e.g., foreign keys, because Mongo expects type ObjectId and you can't give just strings. And no you can't convert them automatically back to ObjectIds before entering the DB again or do other clever type hacks.

This is even more sad, considering that Mongo's native driver has excellent TS support. You might think that these are just nuanced problems of some users, but no, it's makes using SvelteKit/SSR + Mongo impossible.

@dhr-nl
Copy link

dhr-nl commented Sep 5, 2022

someone will create a sanitize-mongo-data package. Not a big deal.

I agree, here's the gist. I could get it fast but it messes up TS types: When using this as standard wrapper all ObjectId props need to be typed as string. Then, you can't save ObjectId props as such anymore, e.g., foreign keys, because Mongo expects type ObjectId and you can't give just strings. And no you can't convert them automatically back to ObjectIds before entering the DB again or do other clever type hacks.

I don't understand why converting back to ObjectId before entering the DB is not possible, intentionally in mongodb you wouldn't want to modify ObjectID properties since they are immutable. You can create new ObjectId from string in an object if you want mongodb to find it based on that object ID.

As for serialization/deserialization, there is the EJSON component from the bson package that mongodb is using that contains a serialize and deserialize function. This works for me at least to solve the issue of getting an error with the _id and keeps the functions of toHexString() etc intact.

Unless I'm misunderstanding something then please correct me

<script lang="ts">
import {EJSON} from  'bson'
import type {PageData} from './$types'

export let data

let deserialized = EJSON.deserialize(data)

console.log(deserialized.serializedValue)
console.log(deserialized.serializedValue._id.toHexString())
</script>
export const load: PageServerLoad = async ({ locals }) => {

	const valueFromDb = db.findOne({})
	const data = await valueFromDb
	if (data !== null) {

	const serializedValue = EJSON.serialize(data)
	
	return {
		serializedValue
		};
	}
};

@mostrecent
Copy link

@dhr-nl thanks for the hint, bson's EJSON which is pretty much the perfect de-/serializer which handles everything (ObjectId, Date, etc.)

@Rich-Harris & @dummdidumm is it technically possible to let the user set the de-/serializer and use EJSON instead of devalue?

@dhr-nl
Copy link

dhr-nl commented Sep 22, 2022

@dhr-nl thanks for the hint, bson's EJSON which is pretty much the perfect de-/serializer which handles everything (ObjectId, Date, etc.)

@Rich-Harris & @dummdidumm is it technically possible to let the user set the de-/serializer and use EJSON instead of devalue?

I think the EJSON over-head and then running it through devalue shouldn't be a real problem, so my example above could be the only boilerplate code required to completely use EJSON on backend and front-end alike. Devalue actually works great for majority of use cases and the additional complexity when de-serializers are customizable might not be worth pursuing it.

@Owez
Copy link

Owez commented Nov 26, 2022

This issue would definately be a nice to have. I'm trying to return a typescript class and I need to convert it into an interface or a plain object when I return it, making me need to make an ugly pojo serialization wrapper for a nice class-based api wrapper.

@thiagomagro
Copy link

For MongoDB users, this issue now affects the possibilities to use Streamed responses. If anyone found a solution, would love to know!

manuel139 added a commit to manuel139/awesome-sveltekit that referenced this issue Sep 4, 2024
…g]/+page.server.ts

fixes error reported in sveltejs/kit#6093
might be fixed upstream in sveltejs/kit#6008
@AlbertMarashi
Copy link

AlbertMarashi commented Oct 11, 2024

@Rich-Harris I think you should take another look at supporting custom class serialisation.

We also have this issue with SurrealDB that uses RecordIds.

The core of the issue here is that there is a type mismatch from the load functions.

The IDE expects a class, but the client receives the JSON format of RecordId instead...

@marcus-sa
Copy link

marcus-sa commented Oct 23, 2024

I would also like to use https://deepkit.io/library/type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature / enhancement New feature or request p2-nice-to-have SvelteKit cannot be used by a small number of people, quality of life improvements, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.