-
-
Notifications
You must be signed in to change notification settings - Fork 145
perf(cli): give CLI windows their own renderer entry (2.00x floor -> 1.04x) #590
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+180
−2
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title></title> | ||
| </head> | ||
| <body style="background:#09090b;margin:0"> | ||
| <div id="root"></div> | ||
| <script type="module" src="/src/cli-main.tsx"></script> | ||
| </body> | ||
| </html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| // @vitest-environment jsdom | ||
| // | ||
| // Le dispatch de `cli-main.tsx` est la seule logique du fichier, et c'est elle qui décide | ||
| // si une fenêtre CLI affiche quelque chose ou reste blanche. Trois des quatre types n'ont | ||
| // jamais été exécutés à la main ; ce test est ce qui les couvre. | ||
| import type React from "react"; | ||
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| vi.mock("./cli/CliExportRunner", () => ({ default: () => "EXPORT" })); | ||
| vi.mock("./cli/CliRecordRunner", () => ({ default: () => "RECORD" })); | ||
| vi.mock("./cli/CliSourcesRunner", () => ({ default: () => "SOURCES" })); | ||
| vi.mock("./cli/CliCaptionsRunner", () => ({ default: () => "CAPTIONS" })); | ||
|
|
||
| import { mountCliWindow } from "./cli-main"; | ||
|
|
||
| /** Une racine factice : on veut savoir CE QUI a été rendu, pas le rendre vraiment. */ | ||
| function fakeRoot() { | ||
| const rendered: React.ReactNode[] = []; | ||
| return { rendered, render: (node: React.ReactNode) => rendered.push(node) }; | ||
| } | ||
|
|
||
| /** Le nom du composant effectivement monté, en descendant à travers `React.StrictMode`. */ | ||
| function mountedName(node: unknown): string | undefined { | ||
| const el = node as { type?: unknown; props?: { children?: unknown } } | null; | ||
| if (!el || typeof el !== "object") return undefined; | ||
| const type = el.type as { name?: string } | string | undefined; | ||
| if (typeof type === "function" && type.name) return type.name; | ||
| if (el.props?.children) return mountedName(el.props.children); | ||
| return typeof type === "string" ? type : undefined; | ||
| } | ||
|
|
||
| describe("mountCliWindow", () => { | ||
| for (const [windowType, expected] of [ | ||
| ["cli-export", "EXPORT"], | ||
| ["cli-record", "RECORD"], | ||
| ["cli-sources", "SOURCES"], | ||
| ["cli-captions", "CAPTIONS"], | ||
| ] as const) { | ||
| it(`monte le runner de ${windowType}`, async () => { | ||
| const root = fakeRoot(); | ||
| await mountCliWindow(windowType, root); | ||
| expect(root.rendered).toHaveLength(1); | ||
| // Le composant mocké rend son propre nom : l'appeler prouve que c'est le bon. | ||
| const name = mountedName(root.rendered[0]); | ||
| expect(name).toBeDefined(); | ||
| const Component = name as unknown; | ||
| void Component; | ||
| // On invoque le type trouvé pour lire la chaîne que le mock rend. | ||
| const found = (function invoke(node: unknown): string | undefined { | ||
| const el = node as { type?: unknown; props?: { children?: unknown } } | null; | ||
| if (!el || typeof el !== "object") return undefined; | ||
| if (typeof el.type === "function") return (el.type as () => string)(); | ||
| return invoke(el.props?.children); | ||
| })(root.rendered[0]); | ||
| expect(found).toBe(expected); | ||
| }); | ||
| } | ||
|
|
||
| it("rend une erreur VISIBLE sur un windowType inconnu, et la signale", async () => { | ||
| const root = fakeRoot(); | ||
| // `drop_console: true` retire les `console.*` de la build de production : le seul | ||
| // signal qui survit est ce qui est rendu. C'est ça que ce test verrouille. | ||
| await expect(mountCliWindow("cli-nonexistent", root)).rejects.toThrow(/unexpected windowType/); | ||
| expect(root.rendered).toHaveLength(1); | ||
| expect(JSON.stringify(root.rendered[0])).toContain("cli-nonexistent"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| // Point d'entrée du renderer pour les fenêtres CLI (`cli-export`, `cli-record`, | ||
| // `cli-sources`, `cli-captions`). | ||
| // | ||
| // POURQUOI IL EXISTE. `main.tsx` importe `App.tsx`, donc tout l'éditeur : le store, les | ||
| // panneaux, les dépendances de l'édition IA. Une fenêtre CLI n'affiche rien de tout ça — | ||
| // elle rend un `<div>` avec une ligne de texte pendant qu'un runner appelle le compositeur | ||
| // natif — mais elle en payait quand même le chargement, avant la première frame et donc | ||
| // DANS l'intervalle chronométré. | ||
| // | ||
| // Mesuré sur cette machine (Mac mini M1) : entre l'exécution du preload et celle du module | ||
| // d'entrée, l'`index.html` de l'éditeur met 3,9 s ; tout ce qui suit (chargement du projet, | ||
| // sondage des dimensions, appel natif) tient en 24 ms. | ||
| import React from "react"; | ||
| import ReactDOM from "react-dom/client"; | ||
|
|
||
| /// Monte le runner correspondant à `windowType` dans `root`. | ||
| /// | ||
| /// Exporté, et prenant sa racine en argument, pour que le dispatch soit testable sans | ||
| /// fenêtre Electron : c'est la seule logique de ce fichier, et c'est elle qui décide si une | ||
| /// fenêtre CLI affiche quelque chose ou reste blanche. | ||
| export async function mountCliWindow( | ||
| windowType: string, | ||
| root: { render: (node: React.ReactNode) => void }, | ||
| ): Promise<void> { | ||
| // Import dynamique et CIBLÉ : une fenêtre d'export ne charge pas le code d'enregistrement. | ||
| switch (windowType) { | ||
| case "cli-export": { | ||
| const { default: R } = await import("./cli/CliExportRunner"); | ||
| root.render( | ||
| <React.StrictMode> | ||
| <R /> | ||
| </React.StrictMode>, | ||
| ); | ||
| return; | ||
| } | ||
| case "cli-record": { | ||
| const { default: R } = await import("./cli/CliRecordRunner"); | ||
| root.render( | ||
| <React.StrictMode> | ||
| <R /> | ||
| </React.StrictMode>, | ||
| ); | ||
| return; | ||
| } | ||
| case "cli-sources": { | ||
| const { default: R } = await import("./cli/CliSourcesRunner"); | ||
| root.render( | ||
| <React.StrictMode> | ||
| <R /> | ||
| </React.StrictMode>, | ||
| ); | ||
| return; | ||
| } | ||
| case "cli-captions": { | ||
| const { default: R } = await import("./cli/CliCaptionsRunner"); | ||
| root.render( | ||
| <React.StrictMode> | ||
| <R /> | ||
| </React.StrictMode>, | ||
| ); | ||
| return; | ||
| } | ||
| default: { | ||
| // Une fenêtre CLI sans type connu est un bug d'appel. Le dire DANS LE DOM, pas | ||
| // seulement dans la console : `vite.config.ts` compile avec `drop_console: true`, | ||
| // donc un `console.error` disparaît de la build de production et il ne resterait | ||
| // qu'une fenêtre blanche que personne ne saurait diagnostiquer. | ||
| const message = `openscreen: unexpected windowType ${JSON.stringify(windowType)}`; | ||
| root.render( | ||
| <pre style={{ color: "#f87171", padding: 16, font: "12px ui-monospace, monospace" }}> | ||
| {message} | ||
| </pre>, | ||
| ); | ||
| throw new Error(message); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Amorçage, conditionné à la présence de la racine : c'est ce qui rend le module importable | ||
| // par un test sans qu'il tente de monter dans un document vide. | ||
| const container = document.getElementById("root"); | ||
| if (container) { | ||
| const windowType = new URLSearchParams(window.location.search).get("windowType") || ""; | ||
| void mountCliWindow(windowType, ReactDOM.createRoot(container)).catch((error) => { | ||
| // Le rendu d'erreur a déjà eu lieu dans le DOM ; ceci n'ajoute qu'une trace en | ||
| // développement (`drop_console` la retire en production, d'où le rendu). | ||
| console.error(error); | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: getopenscreen/openscreen
Length of output: 22133
🤖 get_repo_knowledge executed:
get_repo_knowledge getopenscreen/openscreen /tmp/coderabbit-repo-knowledge/getopenscreen-openscreen-086fd783/conventions /tmp/coderabbit-repo-knowledge/getopenscreen-openscreen-086fd783/learningsLength of output: 6810
🏁 Script executed:
Repository: getopenscreen/openscreen
Length of output: 2141
🏁 Script executed:
Repository: getopenscreen/openscreen
Length of output: 2790
Add tests for every CLI dispatch branch.
Cover
cli-export,cli-record,cli-sources,cli-captions, and the unexpected-value branch. Assert that each supported value mounts its matching runner. The repository convention requires tests for every new behavior in the same package, and no adjacent test coverssrc/cli-main.tsx.🤖 Prompt for AI Agents
Source: Coding guidelines