search

onlook-dev / onlook

The open source, local-first Figma for React. Design directly in your live React app and publish your changes to code.
https://onlook.dev
Apache License 2.0
3.71k stars 227 forks source link

[bug] data-onlook-id and data-onlook-unique-id is not stable and consistent #501

Open Kitenite opened 1 month ago

Kitenite commented 1 month ago

Describe the bug

Weare doing DOM manipulation in real-time and map to AST. The AST is currently hydrated with data-onlook-id which is computed based on the file and line-col location of the AST node.

While this works fine for the initial implementation, in real-time updates of the code, the code can shift location which could cause discrepancies between the code node that is detected on the DOM and the actual version of the code.

There are a few requirements to this:

  1. Create a more stable ID
  2. Make the ID persistent across the DOM across updates

Suggestion:

  1. Statically assign a unique ID to the AST nodes while Onlook is running and clean up the IDs afterward. This will ensure that the IDs are stable across moves. The drawback is that the codebase is polluted while Onlook is running.
  2. Compute a consistent ID based on some signature of the node itself such as attributes and contents. If this is possible, it would be a better solution. However, this may not always be possible. (For example, 2 empty divs have no differences between them).
tasbi03 commented 6 days ago

hey , can you assign me this ?

Kitenite commented 6 days ago

@tasbi03 , sure thing. This is quite a core/ambiguous problem so happy to help reach a solution :)

tasbi03 commented 4 days ago

Hi @Kitenite,

I'm working on stabilizing the data-onlook-id and data-onlook-unique-id attributes. Currently, I’m exploring the following approach and would appreciate your input:

Analyzing index.ts in the CodeManager class to understand how selectors reference TemplateNode instances. My initial thoughts are to modify or assign unique, stable IDs to TemplateNode instances, as this may help ensure consistency across code changes. Exploring New ID Generation Methods: I’m considering using content-based or session-unique identifiers for stable ID references, particularly within functions like getAnyTemplateNode and findNodeInstance. I’d likely implement these in AstManager and AstRelationshipManager methods (processNodeForMap, setTemplateRoot, etc.) to maintain stable references. Could you confirm if this direction aligns with your vision for stabilizing these IDs? Also, if there are alternative approaches you’d recommend, please let me know.

Finally, do you have a preferred way to communicate, such as Discord or email, where we can discuss this in more detail?

Thank you !

Kitenite commented 3 days ago

@tasbi03, this is a good starting point! I filmed a quick walkthrough of how the IDs work. We have a data-onlook-id which is based completely out of the element's location. This is actually a TemplateNode serialized. Here's an example of it being created at build time. This is used to map to the element's location in code.

The data-onlook-unique-id is being generated at runtime in the DOM. This is the one that should be more stable. The problem is that it's being regenerated when the DOM refreshes each time. Here's an example of the ID being generated.

We use this data-onlook-unique-id as the CSS selector to do all the element manipulation and to map it to the corresponding TemplateNode in the AstManager you mentioned. It is randomly generated at the moment. If this ID can be more stable over sessions, this would be ideal. Keeping in mind that due to the nature of our manipulation, the content of the element can change and its location can also change. The stability would need to take these into account.

Discord would be a good place to chat. I'm happy to call or email as well but we can DM there :)