surrealdb / surrealdb.js

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

bugfix: Fix the escaping in the built-in ID generation functions #275

Closed tai-kun closed 1 month ago

tai-kun commented 1 month ago

Thank you for the wonderful project.

What is the motivation?

The RecordId class converts built-in generation functions into strings. For example:

const rid = new RecordId("tb", "rand()");
console.log(JSON.stringify(rid)); // "tb:⟨rand()⟩"

However, it should ideally be like this:

const rid = new RecordId("tb", "rand()");
console.log(JSON.stringify(rid)); // "tb:rand()"

// Manually escape built-in generation functions.
const rid = new RecordId("tb", "⟨rand()⟩");
console.log(JSON.stringify(rid)); // "tb:⟨rand()⟩"

What does this change do?

When escaping RecordIdValue, we will not escape rand(), ulid(), or uuid().

For this implementation, the source code I referred to was:

What is your testing strategy?

I added these test suites to tests/unit/jsonify.ts and updated the snapshots.

Is this related to any issues?

N/A

Have you read the Contributing Guidelines?

kearfy commented 1 month ago

Hey @tai-kun! When you pass a string to the RecordId class, it will be treated as such and not as a generate record id. To support such behaviour, we would also need to treat certain strings special, which is exactly what we are trying to move away from. Additionally, generate record ids are currently not supported by the CBOR protocol:

You are able to do this with the StringRecordId class however, which you can use to pass a custom string record id to SurrealDB. To safely construct custom string record ids, we could rather consider to expose the escape_ident method. You could then:

const rid = new StringRecordId(`${escape_ident("tb-name")}:rand()`);

I hope that helps!