surrealdb / surrealdb.js

SurrealDB SDK for JavaScript
https://surrealdb.com
Apache License 2.0
272 stars 45 forks source link

feature: add optional `flatMode` flag to flatten surrealdb responses to plain js objects #244

Closed oskar-gmerek closed 2 months ago

oskar-gmerek commented 2 months ago

What is the motivation?

Switching from JSON to CBOR and the subsequent introduction of classes such as RecordId significantly lowers the DX in its current form, for example, when working with frameworks like SvelteKit. SvelteKit only supports data transmission between the server and the client in JSON format, and attempting to send the current response from SurrealDB will result in an error Error: Data returned from 'load' while rendering / is not serializable: Cannot stringify arbitrary non-POJOs.

The toJSON() method available in the RecordId class solves this problem, but it does not improve DX because before any data can be sent, it must first be destructured and the method used in all occurrences of RecordId.

What does this change do?

This change introduces an optional flatMode flag in the constructor of the Surreal class. When flatMode: true, all responses from SurrealDB that may contain RecordId are recursively flattened into a JavaScript object format via the flatten() function.

Creating new instances: Regular: new Surreal() flatMode: new Surreal({ flatMode: true })

Example Outputs: Regular:

{
   id: RecordId {
      tb: "person", 
      id: 1
   },
   firstname: "John",
   lastname: "Doe",
   age: 30,
},

flatMode:

{
   id: {
      tb: "person", 
      id: 1
   },
   firstname: "John",
   lastname: "Doe",
   age: 30,
},

What is your testing strategy?

I added tests based on existing ones, each test includes the suffix - flatMode. I also compared the performance between the query and query - flatMode tests, which show a performance difference of less than 1ms.

Is this related to any issues?

247

Related to, but this is partial or alternative solution for #241 #242

Also I saw related messages on Discord (but this is partial or alternative solution):

Zrzut ekranu 2024-04-24 o 11 44 02 Zrzut ekranu 2024-04-24 o 11 31 15 Zrzut ekranu 2024-04-28 o 19 33 15

To do

Have you read the Contributing Guidelines?

dodanex commented 2 months ago

@oskar-gmerek agree, the ID should be in the root of the object, not nested!

abrisene commented 2 months ago

+1 - I have abstraction around instantiating a surreal client in 0.11, but the changes to the ids makes migration more complicated than I'd like.

It would be nice to have this option, which would make it easier to migrate more gradually.

Ideally I'd want to have the option to flatten on the query methods themselves since cbor makes it much easier to use complex ids, but changing the signature of each of the query methods seems like a bad idea.

dodanex commented 2 months ago

My opinion is that flat mode should be the default mode:

Creating new instances: Regular with flat mode: new Surreal() With flatMode off: new Surreal({ flatMode: false })

What do you think @kearfy , @oskar-gmerek ?

kearfy commented 2 months ago

Hey @oskar-gmerek! Thanks for working on this PR. I don't think we should support global config options like this, we could however improve the toJSON methods in such a way that you can simply do

await db.select('table-name').then(r => JSON.stringify(r))

And get roughly the same result back as the previous versions of the library. It sounds quite trivial to construct a string RecordID outside the rust library, but it's actually not. Take tb: table:name and id something-else as an example:

"table:name:something-else"

You now end up with an invalid record id, as the two ident parts are not escaped. Now let's say that you not send this to SurrealDB, but you would have RecordId.fromString("table:name:something-else"), it would also result in something else where tb is table and the id part is name:something-else.

What I'm boiling down to is that, yes, we can do this, however it would add the cost of using regex which at scale can massively degrade performance. Ideally implementors would adjust to the new library and take advantage of it's understanding of datatypes native to SurrealQL.

I'll close this PR, but I'll see what I can do to add in a JSON compatibility mode. It will however carry a price tag on performance, and you won't be able to interchange some of this values back to the server without manually converting them back to these new representations of native SurrealQL datatypes, which includes RecordId, UUID, Datetime, Duration, Decimal, BigInt, Table and Geometries.

kearfy commented 2 months ago

Hey all! As you can see, I just merged a PR which allows you to get a JSON-like representation of the responses the way that SurrealDB would also display JSON-like responses.

Here's an example of what you'll be able to do in the next beta:

import { StringRecordId, jsonify } from 'surrealdb.js';

// Use the selection, creation and altering methods with string record ids
const res = await db.select(new StringRecordId("person:john"));

// You will get back a response with native values
// However will `jsonify()` or `JSON.stringify()` you will get back a JSON representation of the result, the way that SurrealDB would also display it
const jsonlike = jsonify(res);
const jsonstring = JSON.stringify(res);

// You can also easily do this in one go
await db
    .select(new StringRecordId("person:john"))
    .then(jsonify);

We added the jsonify() method just in #259. It does a basic JSON stringify and parse. We will investigate if something custom can be more performant and will catch all edge cases. Currently the added benefit of the jsonify() method is that it will transform the types of the passed value to represent the actual JSON output.