Sync snippet Markdown live preview with mid-edit renames

skkzshrequested to merge
gitlab-community/gitlab-org/gitlab:skkzsh/snippet-preview-rename into master

What does this MR do and why?

Follow-up to !246399 (comment 3659462619), as part of Snippet file editor should support preview (#602250).

Also closes Docs: Update snippets page for preview (#625426 - closed).

Accepted limitation in !246399 (merged)

Support Markdown live preview for snippet file ... (!246399 - merged) only registered the Markdown live preview extension on initial load, so the extension doesn't follow mid-edit renames:

  • Renaming .md to .rb leaves Preview Markdown in the right-click menu
  • Renaming .rb to .md doesn't add Preview Markdown to the menu

Approach

Watch isMarkdown, which is derived from blob.path, and install or uninstall the extension on each rename.

Newly added files

This also makes the live preview available for newly added files (via Add another file or New snippet): those start out unnamed, and the preview kicks in once the file gets a Markdown file name.

A single definition of "markdown"

The review suggested a patch that detects renames through Monaco's onDidChangeModelLanguage, the same mechanism the single file editor uses. However, the single file editor has a bug (Live preview is driven by two competing definit... (#619238 - closed)) where loading and unloading are driven by two different definitions of "markdown": GitLab's file extension list on initial load, and Monaco's language detection on rename.

This MR doesn't use onDidChangeModelLanguage. Instead, it drives everything off the file name through isMarkdownFilePath, the same check used for initial registration, so there is a single source of truth. This matches "Possible fixes A" in Live preview is driven by two competing definit... (#619238 - closed). Unify the definitions of "Markdown" behind edit... (!253025 - merged) does the same for the single file editor.

Also renames hasMarkdownExtension to isMarkdownFilePath. This MR introduces markdownExtension to hold the installed editor extension, next to which the old name reads like a check for that, rather than a check on the file name.

Races between renames and the async load

The extension is lazy-loaded through a dynamic import, so a rename can race with the load. Each case is handled with a guard, following the same approach as Fix races between Markdown extension loading an... (!251306 - merged), which fixed similar races in the single file editor.

For example, renaming away from Markdown while the import is still in flight:

---
title: Rename to non-Markdown while the import is in flight
config:
#  theme: default
  fontSize: 14
---

sequenceDiagram
    autonumber
    actor User
    participant SnippetBlobEdit
    participant Import as dynamic import
    participant Editor

    Note over SnippetBlobEdit: editing foo.md
    Editor-->>SnippetBlobEdit: editor ready
    SnippetBlobEdit->>+Import: start loading the Markdown extension

    rect
        note right of User: race window — the load is still in flight
        User->>SnippetBlobEdit: rename to foo.rst
        Note over SnippetBlobEdit: watch fires (isMarkdown = false) —<br>uninstall is a no-op (markdownExtension is null)
    end

    Import-->>-SnippetBlobEdit: import resolves
    Note over SnippetBlobEdit: guard: !isMarkdown → abort
    SnippetBlobEdit--xEditor: use() is never called
Rename to Markdown before the editor is ready 
---
title: Rename to Markdown before the editor is ready
config:
#  theme: default
  fontSize: 14
---
sequenceDiagram
    autonumber
    actor User
    participant SnippetBlobEdit
    participant Import as dynamic import
    participant Editor

    rect
        note right of User: race window — the editor component has not mounted yet
        User->>SnippetBlobEdit: rename foo.rst → foo.md
        Note over SnippetBlobEdit: watch fires (isMarkdown = true) —<br>guard: editor is null → no-op
    end

    Editor-->>SnippetBlobEdit: editor ready
    Note over SnippetBlobEdit: store the instance,<br>re-sync with the latest isMarkdown
    SnippetBlobEdit->>+Import: start loading the Markdown extension
    Import-->>-SnippetBlobEdit: import resolves
    SnippetBlobEdit->>Editor: use() — installed normally
Rename back to Markdown while the import is in flight 
---
title: Rename back to Markdown while the import is in flight
config:
#  theme: default
  fontSize: 14
---

sequenceDiagram
    autonumber
    actor User
    participant SnippetBlobEdit
    participant Import as dynamic import (shared)
    participant Editor

    Note over SnippetBlobEdit: editing foo.md
    Editor-->>SnippetBlobEdit: editor ready
    SnippetBlobEdit->>+Import: 1st install starts loading

    rect
        note right of User: race window — two install calls, one shared load
        User->>SnippetBlobEdit: rename to foo.rst
        User->>SnippetBlobEdit: rename back to foo.md
        SnippetBlobEdit->>Import: 2nd install awaits the same in-flight load
    end

    Import-->>-SnippetBlobEdit: import resolves — both awaits resume in call order 
    SnippetBlobEdit->>Editor: 1st: use() — markdownExtension is set
    Note over SnippetBlobEdit: 2nd: guard — markdownExtension already set → abort

🔴 RED → 🟢 GREEN cycles

  1. Add specs for syncing the live preview with mid-edit renames — 🔴 RED.
  2. Watch isMarkdown and install or uninstall the extension on each rename, turning it 🟢 GREEN.
  3. Add specs asserting that unuse is skipped when no extension is installed — 🔴 RED.
  4. Guard install and uninstall against mismatched extension state, turning it 🟢 GREEN.
  5. Add a spec for renaming to a markdown file before the editor is ready — 🔴 RED.
  6. Skip syncing until the editor is ready, turning it 🟢 GREEN.
  7. Add a spec for renaming away from markdown while the import is still in flight — 🔴 RED.
  8. Skip installing the extension once the import resolves if the file is no longer markdown, turning it 🟢 GREEN.
  9. Add specs for two loads racing after a rename round-trip — 🔴 RED.
  10. Skip extensions that a racing load has already installed, turning it 🟢 GREEN.

Related work

Related issues and MRs on the single file editor side:

Screen recordings

Manually verified in GDK-in-a-box:

How to set up and validate locally

  1. Start a New snippet and name the file README.md.
  2. Write some Markdown content (e.g. # Title).
  3. Right-click inside the editor, select Preview Markdown, and confirm a preview panel renders the content next to the editor.
  4. Remove the .md suffix from the file name, right-click again, and confirm Preview Markdown is gone from the menu (and the preview panel closes).
  5. Add the .md suffix back, right-click again, and confirm Preview Markdown works again.

Notes

  • I don't work on the frontend regularly — a careful look at this approach would be very helpful.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by skkzsh

Merge request reports