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

RFC: useSerialized$/createSerialized$ + SerializeSymbol #204

Open
wmertens opened this issue Jan 7, 2025 · 17 comments
Open

RFC: useSerialized$/createSerialized$ + SerializeSymbol #204

wmertens opened this issue Jan 7, 2025 · 17 comments
Assignees
Labels
[STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation

Comments

@wmertens
Copy link
Member

wmertens commented Jan 7, 2025

Champion

@wmertens

What's the motivation for this proposal?

Problems you are trying to solve:

  • third-party libraries with non-serializable objects

Proposed Solution / Feature

What do you propose?

  1. a Symbol prop, SerializerSymbol, you can attach to an object that will serialize the object
  2. a Signal that holds the object and will lazily recreate it when needed

Code examples

Providing its own data:

class MyCustomSerializable {
  constructor(public n: number) {}
  inc() {
    this.n++;
  }
}
const Cmp = component$(() => {
  const custom = useSerialized$({
    deserialize: (n) => new MyCustomSerializable(n),
    serialize: (obj) => obj.n,
    initial: 3
  );
  return <div onClick$={() => custom.value.inc()}>{custom.value.n}</div>;
});

Getting the data from a signal:

import {makeThing} from 'things'

const serializer = (thing) => thing.toJSON()

const Cmp = component$(() => {
  const myData = useSignal(3);
  const custom = useSerialized$(() => ({
    deserialize() { return makeThing(myData.value); }
    update(current) {
      current.update(myData.value);
      return current;
    }
  }));
  return <div onClick$={() => custom.value.doThing()}>{custom.value.stuff}</div>;
});

In detail

  • you must provide a deserialize function that gets the data and the current object. It gets called to create the value the first time, to update when signals change, and to recreate the value with serialized data
  • useSerialized$() {deserialize, initial, update, serialize} that will be executed on the server as well as the browser to create the object, and it returns a Signal
    • the Signal works exactly like ComputedSignal, except that it will always be marked to-calculate in the browser, and it passes the serialized value + the previous value to the compute function
  • bonus: you can add [SerializeSymbol]: (obj) => serialize(obj) to any object that needs custom serialization (but not deser)
    • note that this can also used on regular objects to do housekeeping before serialization
    • Promise results are awaited so you can even e.g. save a reference number to a db

PRs/ Links / References

QwikDev/qwik#7223

@github-project-automation github-project-automation bot moved this to In Progress (STAGE 2) in Qwik Evolution Jan 7, 2025
@github-actions github-actions bot added [STAGE-2] incomplete implementation Remove this label when implementation is complete [STAGE-2] not fully covered by tests yet Remove this label when tests are verified to cover the implementation [STAGE-2] unresolved discussions left Remove this label when all critical discussions are resolved on the issue [STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation labels Jan 7, 2025
@wmertens wmertens changed the title RFC: useSerialized$/createSerialized$ + SerializerSymbol RFC: useSerialized$/createSerialized$ + SerializeSymbol Jan 7, 2025
@GrandSchtroumpf
Copy link

I think this is a great addition to Qwik. On a devX point of view I find it a little difficult to understand because serialize & deserialize don't have the same mecanism (serialize -> SerializeSymbol / deserialize -> useSerialized$).
Maybe something like that :

const custom = useSerialized(new MyCustomClass(), {
  serialize: $((instance) => instance.toJSON()),
  deserialize: $((json) => MyCustomClass.fromJSON(json))
});

Also scoping it into a use hook limit its usage inside components, while it could be useful outside too. It won't be lazy, but as long as the dev is aware of that, I think it would be a good tool.

import { MemoryDB } from 'third-party';
export const db = serialize(new MemoryDB(), {
  serialize: (instance) => instance.getState(),
  deserialize: (state) => new MemoryDB(state)
});

// Create API
export const addFoo = $(async (foo: Foo) => {
  await db.store('foo').add(foo);
  // Do additionaly logic here
})

@wmertens
Copy link
Member Author

wmertens commented Jan 7, 2025

The first interface can work (the serializer doesn't need to be qrl but can be). I think I'd still like to keep the symbol interface for cleanup before serialization etc.

For the use issue, there's also createSerialized$ which you can use anywhere, so that's not an issue.

@wmertens
Copy link
Member Author

wmertens commented Jan 7, 2025

Oh but the creation needs to be a function and the deserializer does the same, so it could be like

const createSerialized$((data) => new MyCustomClass(data), (obj) => obj.toJSON())

, with the serializer optional.

EDIT: actually this would mean keeping a a WeakMap for the serialization functions, instead of the single prototype object with the serializer. So I'm not a fan.

@GrandSchtroumpf
Copy link

I'm not sure to understand how the createSerialized$ works. Could you share a compete example ?
Let's say I want to create a MemoryDB instance on the server and pick it up in the client, how would it work ?

import { MemoryDB } from 'third-party';

// Create global instance that I can access in both server & client
export const db = ... ;

// Use instance in either server or client
export const addFoo = $(async (foo: Foo) => {
  // db.add(...)
})

@wmertens
Copy link
Member Author

wmertens commented Jan 7, 2025

That won't work, the db connection won't be initialized in the client.

addFoo would be a method of an object you create, and you'd use it like signal.value.addFoo, and it would create its db connection if needed.

@GrandSchtroumpf
Copy link

Let's say MemoryDB is something that doesn't require any connection, something dummy, but unserializable, like that :

class MemoryDB {
  state = {};
  add(store, value) {
    const id = randomUUID();
    this.state[store] ||= {};
    this.state[store][id] = value;
  }
  get(store, id) {
    return this.state[store][id];
  }
  ...
}

How can I use createSerialized$ to serialize it on the server and deserialize it on the client ?
(I understand I could achieve that with useStore, it's just to have some code example to better understand how it would work.)

@wmertens
Copy link
Member Author

wmertens commented Jan 7, 2025

@GrandSchtroumpf something like

class MemoryDB {
  constructor(public state = {}) {}
  add(store, value) {
    const id = randomUUID();
    this.state[store] ||= {};
    this.state[store][id] = value;
  }
  get(store, id) {
    return this.state[store][id];
  }
  [SerializeSymbol]() { return this.state }
  ...
}

const dbSig = createSerialized$((data) => new MemoryDB(data))

@GrandSchtroumpf
Copy link

In a previous comment you mentioned we can use createSerialized$ to work outside a component.

For the use issue, there's also createSerialized$ which you can use anywhere, so that's not an issue.

How can I use it ? Like that ?

import { MemoryDB } from 'third-party';

// Create global instance that I can access in both server & client
export const dbSig = createSerialized$((data) => {
  const instance = new MemoryDB(data);
  instance[SerializeSymbol] = () => instance.state;
  return instance
});

// Use instance in either server or client
export const addFoo = $((foo: Foo) => {
  dbSig.value.add('foo', foo);
})

@wmertens
Copy link
Member Author

wmertens commented Jan 7, 2025

Yes correct, I edited the example, I used use by accident.

@thejackshelton
Copy link
Member

thejackshelton commented Jan 7, 2025

The use case of useSerialized$ is especially valuable for integrating vanilla JS libraries with Qwik.

The current challenge with libraries like TanStack Table is that they have a vanilla JS core that creates complex objects with methods. These objects often become non-serializable in Qwik's resumable architecture, leading to runtime errors when trying to pause/resume the application state. (it does not recover the right info, at least in v1)

For example, with TanStack Table, they create table instances with methods like:

const table = createTable({
  data: [],
  columns: []
})

// Methods on the table object can cause serialization issues
table.getRowModel()
table.getSortedRowModel()

The proposed useSerialized$ would let us properly serialize/deserialize these instances by:

  1. Defining how to serialize the core state
  2. Providing a way to reconstruct the full object with methods in the browser
const tableSignal = useSerialized$((data) => {
  const table = createTable(data)
  table[SerializeSymbol] = () => table.getState() // Serialize core state
  return table
})

This would be a huge improvement over current workarounds like:

  • Not serializing at all (noSerialize around everything)
  • Manually managing serialization (external to what the framework understands, similar to imperative browser changes with the DOM and the framework)
  • Restructuring code to avoid serialization

image
image

@wmertens wmertens removed [STAGE-2] incomplete implementation Remove this label when implementation is complete [STAGE-2] not fully covered by tests yet Remove this label when tests are verified to cover the implementation labels Jan 8, 2025
@wmertens wmertens moved this from In Progress (STAGE 2) to To Review (STAGE 3) in Qwik Evolution Jan 8, 2025
@mhevery
Copy link

mhevery commented Jan 8, 2025

@wmertens

// existing propsal
const custom = useSerialized$(new MyCustomClass(), {
  serialize: $((instance) => instance.toJSON()),
  deserialize: $((json) => MyCustomClass.fromJSON(json))
});

// mhevery proposal
class MyClass {
  static serialize: ((instance) => instance.toJSON())
  static deserialize: ((json) => MyCustomClass.fromJSON(json))
}

const custom = useSerialized$(MyClass, optionalInitialSerializableValue);
// non class syntax
const custom = useSerialized$({
  serialize: ((instance) => instance.toJSON()),
  deserialize: ((json) => MyCustomClass.fromJSON(json))
}, optionalInitialSerializableValue);

I think the above syntax is cleaner and does not require that the object has to be a class.

@shairez shairez moved this from Backlog to In progress in Qwik Development Jan 8, 2025
@shairez shairez added the [STAGE-2] incomplete implementation Remove this label when implementation is complete label Jan 8, 2025
@wmertens
Copy link
Member Author

wmertens commented Jan 9, 2025

@mhevery ok, I like it, how about this:

type SerializationOptions<T, S> = {
  // No need for `serialize` in the browser
  serialize?: false | (obj: T) => S | Promise<S>,
  // If you rerun due to scope capture, the previous will be in the second argument
  deserialize: (...args: [data: S | undefined, previous: undefined] | [data: undefined, previous: T]) => T
  // put the initial data inside the segment too
  initialData?: S | () => S
})
declare const createSerialized$: (opts: SerializationOptions) => Signal<T>

const mySig = createSerialized$({
  serialize: isServer && o => o.toJson(),
  deserialize: d => new MyObj(d)
})

// This will optimize to:
const mySig = createSerializedQrl(qrl(() => import('./opts.js'), 'opts'))
// ...and ./opts.js in client:
export default opts = {
  serialize: false,
  deserialize: d => new MyObj(d)
}

We could even special-case it in the optimizer to strip the serialize in the browser.

The SerializedSignal then stores the serializer function. The advantage is that if the value gets stored outside of the signal, serialization will fail, which is a good thing.

So, 1), is this API ok?

That said, I'd still like to keep the NoSerializeSymbol and SerializerSymbol, because they allow using memory more efficiently as well as cleaning up structures before serialization.
2) Is that OK?

@mhevery
Copy link

mhevery commented Jan 10, 2025

Looks reasonable. The only comment I have is that Browser does need serialization if you want to use server$, so I would not make that optional. The serializer is usually small so i don't think it is worth making it optional

@wmertens wmertens removed the [STAGE-2] incomplete implementation Remove this label when implementation is complete label Feb 6, 2025
@wmertens wmertens removed the [STAGE-2] unresolved discussions left Remove this label when all critical discussions are resolved on the issue label Feb 6, 2025
@wmertens wmertens moved this from In progress to Waiting For Review in Qwik Development Feb 6, 2025
@shairez shairez moved this from Waiting For Review to In progress in Qwik Development Feb 11, 2025
@shairez shairez moved this from In progress to Waiting For Review in Qwik Development Feb 11, 2025
@ianlet
Copy link

ianlet commented Feb 13, 2025

This approach seems interesting because it allows you to adapt some libraries even if you are not a maintainer of that library (thus don't have write access to the repo). It will unload the actual library maintainers and allow us to grow the Qwik ecosystem more rapidly.

Questions that occurred while trying it:

  1. If we provide initial data and the deserialize function receives an undefined data how should we handle it? Is that possible? The function signature indicates that it could be the case as of now. See this example: https://github.com/ianlet/qwik-custom-serder-scope-issues/blob/main/src/components/qwik-mapbox.tsx#L49
  2. What would be the lifecycle of that new serialization hook (in regards to tasks and cleanups)?

Feedback

  • When providing initial data, the deserialize function argument data could be undefined so we are forced to ! everything, which isn't a very nice DX.
  • I'm not sure I understand why there should be a previous argument in the deserialize function. See this comment feat: custom serialization qwik#7223 (comment).

Other considerations:

  • How could we handle non-serializable params (as props or inside a QRL)? In that example, mapbox-gl-js heavily uses classes to type their objects (e.g. LngLatLike, Point, etc.). So we can't directly use those types as component props and this createSerialized$ wouldn't solve that.

@wmertens
Copy link
Member Author

@ianlet

  1. There are three scenarios: initial run, deserialize run, and update run.
    • Initial is when it wasn't made before, and it gets initialData, or undefined if missing.
    • Deserialize is when it was made on SSR, then seriailized. It gets the result of serialize(), or undefined if missing.
    • Update is when you capture reactive scope and the deserialize needs to run again. data will be undefined and previous (I'm renaming it to current) will be curren the deserialized object.
  2. It's exactly the same as computed signals, meaning it's not attached to a component, meaning no cleanup. If you need cleanup, use a task.

I'll try to make the types somewhat cleaner, but right now it's not very happy about it. Before, I was passing either initial | serialized or current in the same argument, which meant you had to check every time as well, which isn't nice either.

Two different callbacks won't work either, because the reactivity records the calling function. However, maybe I can wrap both, hmm. Interesting idea.

@wmertens
Copy link
Member Author

@ianlet good idea, looks way better now. I added update and also changed the type so it pretends it's not serializeable, and to capture scope you must use a function that returns the config.

For your mapbox example I hope that fixes everything, also, are you really creating the mapbox on the server, is that useful?

For the props you'll still have to use noSerialize, but you could also wrap the prototypes so they have [NoSerializeSymbol].

@ianlet
Copy link

ianlet commented Feb 14, 2025

For your mapbox example I hope that fixes everything, also, are you really creating the mapbox on the server, is that useful?

I don't think it is, actually. Just wanted to experiment with an actual object and that's the only NoSerialize we had in our project 😅 Though currently as we have to wake up Qwik in the client to set it up, it would still be an interesting approach to be able to setup the whole map and that users can start using it without useVisibleTask or any other mechanisms that could introduce extra delays.

Now that I sidetracked a bit, back to the actual RFC!

Great work for introducing the update callback 😮 Last thing for me would be to hear others about naming: is it clear/intuitive to go with update or something else would even be better? I naturally mentioned update earlier but would still be curious to hear more people share about that before we move on with this API (and make it harder to change in the future).

Maybe I could ask about it the #qwik-v2 channel in discord?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[STAGE-3] docs changes not added yet Remove this label when the necessary documentation for the feature / change is added [STAGE-3] missing 2 reviews for RFC PRs Remove this label when at least 2 core team members reviewed and approved the RFC implementation
Projects
Status: Waiting For Review
Status: To Review (STAGE 3)
Development

No branches or pull requests

6 participants