y-codemirror.next icon indicating copy to clipboard operation
y-codemirror.next copied to clipboard

Reading ySyncFacet dynamically.

Open a3626a opened this issue 3 years ago • 2 comments

Is your feature request related to a problem? Please describe.

Hi, I am working on this PR https://github.com/jupyterlab/jupyterlab/pull/13093 . There is a need to dynamically update YSyncConfig after the initialization of EditorView. I think ySyncFacet is a nice place to do this. However YSyncPluginValue reads this facet during construction, and caches its value. So changing ySyncFacet after the initialization of EditorView has no effect on YSyncPluginValue.

I am changing ySyncFacet by using CodeMirror compartment.

packages/codemirror/src/editor.ts

...
    const ySyncFacetCompartment = new Compartment();
...
    this._editor = Private.createEditor(
      host,
      fullConfig,
      this._yeditorBinding,
      this._editorConfig,
      [
        this._markField,
        Prec.high(domEventHandlers),
        updateListener,
        translation,
        // might need some precedence setup here.
        ySyncFacetCompartment.of(
          ySyncFacet.of(
            new YSyncConfig(
              this._yeditorBinding?.text,
              this._yeditorBinding?.awareness
            )
          )
        ),
      ]
    );
...
    // every time the model is switched, we need to re-initialize the editor binding
    this.model.sharedModelSwitched.connect(() => {
      this._initializeEditorBinding();

      // it does change ySyncFacet. However YSyncPluginValue already cached old value.
      this._editor.dispatch({
        effects: ySyncFacetCompartment.reconfigure(
          ySyncFacet.of(
            new YSyncConfig(
              this._yeditorBinding?.text,
              this._yeditorBinding?.awareness
            )
          )
        )
      });
    }, this);
...

Describe the solution you'd like

I think just not caching the facet value can solve the problem.

Describe alternatives you've considered

No

Additional context

https://github.com/jupyterlab/jupyterlab/pull/13093

a3626a avatar Sep 16 '22 09:09 a3626a

This should work when you create a compartment of all the y-codemirror plugins, right? Just a quick fix.

Ideally, it should be possible to create a compartment of only the facet. I'll investigate how much effort this would take.

dmonad avatar Sep 16 '22 10:09 dmonad

This should work when you create a compartment of all the y-codemirror plugins, right? Just a quick fix.

I think so. Jupyterlab uses compartment to dynamically change codemirror plugins, including configurations like language, them, tab size and so on.

  • https://github.com/jupyterlab/jupyterlab/blob/master/packages/codemirror/src/editorconfiguration.ts

Additionally, in the documentaion:

Facet values are only recomputed when necessary, so you can use an object or array identity test to cheaply check whether a facet changed.

So it is safe (no performance degradation) not to cache the facet output value.

a3626a avatar Sep 19 '22 02:09 a3626a