Apply the Monaco theme before the editor mounts

[mjuarros]

Description

The CodeEditor previously defined its custom Monaco theme in onMount, after Monaco had already created the editor and attempted to apply the named theme. On the first render, Monaco therefore fell back to its default light theme and the editor appeared white.

This change registers graph-explorer-light in beforeMount, before Monaco creates the editor and applies the theme prop. It preserves Monaco's lazy loading and invokes any caller-provided beforeMount callback after theme registration.

How to read

1. packages/graph-explorer/src/components/CodeEditor.tsx registers the theme before editor creation while preserving caller callbacks.
2. packages/graph-explorer/src/components/CodeEditor.test.tsx covers theme registration order and theme configuration.

Validation

• pnpm checks
• pnpm test CodeEditor.test.tsx

Related Issues

Fixes #1645

Check List

• I confirm that my contribution is made under the terms of the Apache 2.0 license.
• I have verified pnpm checks passes with no errors.
• I have verified pnpm test CodeEditor.test.tsx passes with no failures.
• I have covered new added functionality with unit tests if necessary.
• Documentation changes are not required.

[kmcginnes]

Looks good, but I had one concern that needs to be verified first.

[kmcginnes]

Thanks for taking this on. I reported the symptom but not the cause, and the seam you landed on is the right one. I traced it through monaco-editor@0.55.1 to confirm: StandaloneThemeService.setTheme falls back to the built-in vs theme when the name is not registered yet, and defineTheme only re-applies when this._theme.themeName === themeName. So the old onMount call registered graph-explorer-light and then declined to apply it, and that editor stayed white for its whole lifetime rather than for a frame. Moving registration into beforeMount puts it ahead of both editor.create and setTheme in the same synchronous block, so there is no point at which the wrong theme is painted. That fully closes #1645.

Destructuring beforeMount out before the {...props} spread is the right shape too, and it happens to fix a trap the old code had: onMount was not destructured and sat before the spread, so any future call site passing onMount would have silently killed theming.

Two things before this can land, both outside the fix itself.

The test file needs a pass. src/setupTests.ts already mocks @monaco-editor/react globally, so the local vi.doMock plus vi.resetModules() is a second, divergent double, and it is the reason every test has to be async with await import("./CodeEditor"). A hoisted vi.mock in this file overrides the global one and lets you import the component normally. Details inline.

Please rebase onto current main. You are three commits behind, and one of them (#2156) bumps monaco-editor to 0.56 and @testing-library/jest-dom to 7, a major. Since this change is entirely a claim about beforeMount timing proved by a happy-dom test, both the behavior and its proof want re-verifying against what CI will actually install. Nothing conflicts textually, so no warning will fire on its own.

One last thing on the PR text. The description still says the fix "moves the theme definition to loader.init() so it is defined once, globally," which is what your first commit did before you replaced it. We squash-merge with commit messages concatenated, so both the title and that explanation would land on main verbatim over a diff that does neither, and the title also gets copied into the release notes. Could you retitle to something like "Apply the Monaco theme before the editor mounts" and update the body to describe the beforeMount mechanism? Worth squashing the two commits while you are there, and dropping the (#1645) suffix from the second subject, since a trailing (#N) means the merging PR number in this repo. Keep Fixes #1645 in the body.

Reviewing this also surfaced a few pre-existing issues in the file and its dependency tree that have nothing to do with your change. I am filing those separately, nothing for you to do here.

[kmcginnes]

Everything from the last round landed, and I checked each one rather than taking the replies on trust. Rebased onto current main with nothing behind, squashed to a single commit, and the title and description now describe the beforeMount mechanism. The hoisted vi.mock with a static import is exactly right, EditorProps on the fake means a signature change upstream will now break the typecheck, and dropping data-theme in favour of mock.calls[0][0] removed the last assertion that was reading the mock's own plumbing.

I ran it locally: pnpm checks clean, and the full suite passes at 220 files and 2673 tests, so the file-level vi.mock is not leaking into anything that relies on the global mock in setupTests.ts.

I also mutation-tested all three tests, since a test that cannot fail is worse than no test. Two of them bite:

• Moving defineTheme back into onMount, the original bug, fails should configure the theme before invoking the caller beforeMount.
• Flipping base to "vs-dark" also fails it, so tightening to objectContaining({ base: "vs", inherit: true }) did the job.
• Deleting the theme prop fails should render the editor with the graph-explorer-light theme.

The third one does not, and that is the one thing left. Details inline.

One knock-on: if the guard goes away, the "How to read" line that says the test file covers "the lazy-loading regression" needs a trim too.

[kmcginnes]

Verified on 4444eac. pnpm checks clean, full suite green at 2672 tests, and I mutation-tested both remaining tests: reverting defineTheme to onMount, flipping base to "vs-dark", deleting the theme prop, and dropping the caller's beforeMount each fail a test. No dead assertions left.

Deleting the eager-init guard was the right call. Thanks for the patience across three rounds on a nine-line fix.