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

Remove document's content from ModelDB #11602

Closed
hbcarlos opened this issue Dec 1, 2021 · 0 comments
Closed

Remove document's content from ModelDB #11602

hbcarlos opened this issue Dec 1, 2021 · 0 comments
Labels
api-change A change that should be accompanied by a major version increase enhancement tag:Real Time Collaboration

Comments

@hbcarlos
Copy link
Member

hbcarlos commented Dec 1, 2021

Follows #11434

I'll be updating this comment until we find the definitive API

Problem

Currently, we have the document's content triplicated in the editor, ModelDB and YJS. I.e., the same content is mirrored in different data structures. This means that every time one of the models changes, we must update all other models to keep them in sync. This approach is very complex and prone to bugs. As commented several times, we need to remove the content from ModelDB and use only the shared models to manipulate it. This will allow us to simplify the existing code. For extension authors, this means that the API will change.

Proposed Solution

The idea is to open multiple PRs to remove attributes from ModelDB and keep them only in the shared models. Some of the changes are:

Remove document's content:

Remove document's metadata:

  • PR: none yet
  • API changes:
    • Document's metadata are stored in ModelDB as an IObservableJSON named metadata. We want to remove this attribute from ModelDB to use its equivalent from the shared model:
    • Regarding the models the change would be readonly metadata: IObservableJSON -> readonly metadata: nbformat.INotebookMetadata
    • Changing the metadata property is a huge change because at the moment the IObservableJSON extends from IObservableMap, this means that we can change and listen to changes in specific attributes of the metadata. With the new implementation, this won't be possible since in the shared models we need to replace the whole metadata object every time we want to add|remove|update an attribute. The reason is to avoid inconsistent states of the metadata, for example when one client is changing the execution count while another is switching the type of the cell to a markdown. The metadata could end up being a markdown cell with the attribute executionCount.

Remove cell's outputs:

  • PR: none yet
  • API changes:
    • Not sure yet.
    • The problem with the outputs is that they are stored in the view and the shared model. Maybe it is better to have the state stored in the shared model and render them every time.
@hbcarlos hbcarlos added api-change A change that should be accompanied by a major version increase tag:Real Time Collaboration labels Dec 1, 2021
Repository owner moved this from Todo to Done in Real Time Collaboration Nov 25, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-change A change that should be accompanied by a major version increase enhancement tag:Real Time Collaboration
Projects
No open projects
Development

No branches or pull requests

1 participant