DiffEditor fix: set correct model before value update

[StefanDBTLabs]

When trying to keep the models in the DiffEditor the code sometimes updated the incorrect model with the incorrect value because it assumes the correct model is loaded. This update checks and sets the correct model for the passed modelpath before allowing updates to be made.

This addresses the issue described in #478

[suren-atoyan]

@StefanDBTLabs thanks for opening PR 🙌 I'll keep you up to date once I check it.

[suren-atoyan]

hey @StefanDBTLabs 👋

I checked your report and I can confirm that there is an issue. At the same time, the issue isn't related to keepOriginalModelPath or keepModifiedModelPath. These properties basically tell the component to keep the models after unmounting. In your case, there is no unmount, you just update the same DiffEditor component. The issue is in tracking path (originalModelPath and modifiedModelPath). Whenever the user provides a new path, we need to create (or get if exists) a new model with the provided path. That part is missing in DiffEditor implementation. Here you can see how it's implemented in the Editor component. So, basically, you need two more useUpdates with originalModelPath and modifiedModelPath instead of checking them whenever modified or original properties are changed - the worst thing here is that in the controlled mode you will do these model-related checks on each keypress; that's not good, these are two unrelated side effects and should be handled with independent useEffects (useUpdates in this particular case).

Revert your changes and add these two useUpdates before disposeEditor:

  // ...

  useUpdate(
    () => {
      if (editorRef.current && monacoRef.current) {
        const originalEditor = editorRef.current.getOriginalEditor();
        const model = getOrCreateModel(
          monacoRef.current,
          original || '',
          originalLanguage || language || 'text',
          originalModelPath || '',
        );

        if (model !== originalEditor.getModel()) {
          originalEditor.setModel(model);
        }
      }
    },
    [originalModelPath],
    isEditorReady,
  );

  useUpdate(
    () => {
      if (editorRef.current && monacoRef.current) {
        const modifiedEditor = editorRef.current.getModifiedEditor();
        const model = getOrCreateModel(
          monacoRef.current,
          modified || '',
          modifiedLanguage || language || 'text',
          modifiedModelPath || '',
        );

        if (model !== modifiedEditor.getModel()) {
          modifiedEditor.setModel(model);
        }
      }
    },
    [modifiedModelPath],
    isEditorReady,
  );

  function disposeEditor(...)

Let me know if this makes sense and thanks a lot for your effort 🙂

[StefanDBTLabs]

thanks for helping to debug this issue.

[suren-atoyan]

merged 🥳 I'll let you know once I publish a new version

[CaptainArni]

Is there any estimate on when the new version will be published?

[suren-atoyan]

@StefanDBTLabs @CaptainArni it's published 🎉 check v4.5.1