Add model version

[davidbrochart]

Closes #138.

[fcollonval]

Thanks @davidbrochart

Should we move forward with this? If so I have two questions:

• What version should we use? I would be in favor of using 1.0.0 and making the next release of the package 1.0.0 as we are going to release JupyterLab 4.0.0 and we have not seen a need for changes here.
• Should we add the version in all cell types instead of just YBaseCell?

[davidbrochart]

IIRC in #138 we were discussing the use of WebSocket subprotocols, but I guess we don't need that now that we can pass the document model version in the DocSessionHandler's reply?

• What version should we use? I would be in favor of using 1.0.0 and making the next release of the package 1.0.0 as we are going to release JupyterLab 4.0.0 and we have not seen a need for changes here.

Sounds good to me.

• Should we add the version in all cell types instead of just YBaseCell?

You mean so that e.g. a Markdown cell and a code cell can have different document versions? If yes, I think so.

[fcollonval]

You mean so that e.g. a Markdown cell and a code cell can have different document versions? If yes, I think so.

Yes

---

Do you have time to push this forward? Or should I do it?

[davidbrochart]

I can do it.

[davidbrochart]

Actually I'm wondering if cells should have a version, since they are not really a document.
The Python API only provides a version for documents (notebook, file...).

[davidbrochart]

My idea was to have a version attribute in ISharedDocument, so that all shared documents must set a version. But since they derive from YDocument which implements ISharedDocument, I had to omit version from YDocument, and then there is this error:

src/ydocument.ts(69,7): error TS2416: Property 'changed' in type 'YDocument<T>' is not assignable to the same property in base type 'Omit<ISharedDocument, "version">'.
  Type 'ISignal<this, T>' is not assignable to type 'ISignal<ISharedDocument, DocumentChange>'.
    Type 'this' is not assignable to type 'ISharedDocument'.
      Property 'version' is missing in type 'YDocument<T>' but required in type 'ISharedDocument'.
src/ydocument.ts(76,7): error TS2416: Property 'disposed' in type 'YDocument<T>' is not assignable to the same property in base type 'Omit<ISharedDocument, "version">'.
  Type 'ISignal<this, void>' is not assignable to type 'ISignal<ISharedDocument, void>'.
    Type 'this' is not assignable to type 'ISharedDocument'.

Any idea?

[davidbrochart]

An alternative would be to have an empty version in YDocument (and nothing in ISharedDocument), but that would not enforce shared documents to have a version at compile-time.

[fcollonval]

Actually I'm wondering if cells should have a version, since they are not really a document.
The Python API only provides a version for documents (notebook, file...).

Yes I agree with you it make sense to only apply it for document types.

An alternative would be to have an empty version in YDocument (and nothing in ISharedDocument), but that would not enforce shared documents to have a version at compile-time.

A solution would be to turn that class to an abstract class with version as abstract field: https://www.typescriptlang.org/docs/handbook/2/classes.html#abstract-classes-and-members

[davidbrochart]

A solution would be to turn that class to an abstract class with version as abstract field

Thanks, I tried to turn YDocument into an abstract class but then it cannot have a create method anymore, since an instance of an abstract class cannot be created. I'm not sure how to create a class of the derived class.

[fcollonval]

I would be in favor of dropping it as anyway that class is not meant to be instantiated.

Pinging @hbcarlos for his opinion.

[hbcarlos]

Yes, we can drop it.

[davidbrochart]

This yarn.lock file is driving me crazy. If I commit what I have on my machine, the CI gets a different file. If I match the CI file, it doesn't work 😠

[hbcarlos]

This yarn.lock file is driving me crazy. If I commit what I have on my machine, the CI gets a different file. If I match the CI file, it doesn't work 😠

I think it was related to the changes in the yarn.lock. This PR should not change that file.

[hbcarlos]

Thanks, @davidbrochart!

[fcollonval]

Thanks @davidbrochart