Skip to content

Commit 8fb9165

Browse files
committed
updated AGENTS.md and CONTRIBUTING.md based on Refactor learnings
1 parent 4b6d8e3 commit 8fb9165

3 files changed

Lines changed: 108 additions & 28 deletions

File tree

AGENTS.md

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,16 @@ Use https://github.com/obsidian-typings/obsidian-typings as a reference when dea
3737

3838
Treat build code and runtime code as one system.
3939

40-
## Project overview
40+
## Project Overview And Tooling
4141

4242
- Target: Obsidian Community Plugin (TypeScript → bundled JavaScript).
43-
- Entry point: `src/main.ts` compiled to `main.js` and loaded by Obsidian.
44-
- Required release artifacts: `main.js`, `manifest.json`, and optional `styles.css`.
45-
46-
## Environment & tooling
47-
48-
- Node.js: use current LTS (Node 22+).
49-
- **Package manager: npm** (required for this sample - `package.json` defines npm scripts and dependencies).
50-
- **Bundler: esbuild** (required for this sample - `esbuild.config.mjs` and build scripts depend on it). Alternative bundlers like Rollup or webpack are acceptable for other projects if they bundle all external dependencies into `main.js`.
51-
- Types: `obsidian` type definitions.
52-
53-
## Linting
54-
55-
- ESLint is preconfigured with `eslint-plugin-obsidianmd` for Obsidian-specific rules.
56-
- Run `npm run lint` to lint the project.
57-
- A GitHub Action automatically lints every commit on all branches.
43+
- Runtime entry point: `src/core/main.ts`, compiled by `rollup.config.mjs` to `dist/main.js`.
44+
- Required release artifacts: `main.js`, `manifest.json`, and `styles.css`.
45+
- Node.js: use Node 22 or newer. Check `node --version` before diagnosing build-tool failures; mixed Node/Corepack installations can produce misleading errors.
46+
- Package manager: npm. Use `npm install` and the scripts in this repository's `package.json`.
47+
- Runtime bundler: Rollup. Do not replace this build with a sample-plugin esbuild setup; Rollup also assembles compressed runtime payloads and merged CSS.
48+
- Primary lint command: `npm run code`. `npm run lint` is broader and may expose unrelated repository backlog.
49+
- Types: Obsidian type definitions plus conservative declarations for intentionally used unpublished APIs.
5850

5951
## Security, privacy, and compliance
6052

@@ -85,6 +77,7 @@ Follow Obsidian's **Developer Policies** and **Plugin Guidelines**. In particula
8577
- Use the existing visibility helpers instead of the native `hidden` property or attribute, which is not reliable in Obsidian's styled UI. Choose `hideElement`/`showElement` or `setComponentVisibility` from `src/utils/styleUtils.ts`, or `setElementHidden`/`setElementDisplay` from `src/utils/htmlUtils.ts` when a boolean/display-oriented API is clearer.
8678
- Search `src/utils/styleUtils.ts` and `src/utils/htmlUtils.ts` before introducing new DOM styling helpers or display classes.
8779
- For a known vault path, use the most specific synchronous lookup: `app.vault.getFolderByPath()` for folders and `app.vault.getFileByPath()` for files. Use `getAbstractFileByPath()` only when either type is intentionally accepted, and avoid adapter-level existence checks when the Vault API already models the target.
80+
- Radix content in the customized Excalidraw package may be rendered through `ObsidianRadixPortal` directly under the owning document's body. A body portal escapes component ancestor selectors and modal stacking contexts. When a trigger is visible but its menu or popover is not, first inspect whether the content mounted behind a modal or lost ancestor-scoped styles. Use a class on the portaled content, a portal-safe selector, and an explicit stacking level when required; validate main-window, popout, click-outside, and Escape behavior.
8881

8982
## Performance
9083

@@ -191,20 +184,44 @@ This project uses a non-trivial Rollup build because startup time, popout-window
191184

192185
The build embeds or injects runtime code for:
193186

194-
- React and ReactDOM
195-
- a JSX runtime shim for compatibility
196-
- the customized `@zsviczian/excalidraw` package
187+
- React, ReactDOM/client, and the official JSX runtime entry points built from the installed npm packages
188+
- the customized `@zsviczian/excalidraw` Obsidian artifact built from Excalidraw's ESM source graph
197189
- `MathjaxToSVG`
198190
- `lz-string`
199191
- selected compressed locale payloads
200192

201193
These payloads are executed or unpacked at runtime. This is intentional.
202194

195+
React and the Excalidraw package are separate payloads. React must not be bundled into the Excalidraw artifact, because the plugin creates a matching private React runtime in every Obsidian window. Mermaid is also intentionally absent from the artifact and is loaded lazily at runtime through Excalidraw Extras. All other required Excalidraw assets are expected to work offline except the deliberately lazy CJK font subsets.
196+
197+
### Two-Repository Excalidraw Workflow
198+
199+
The customized component lives in the sibling `zsviczian/excalidraw` repository. When both repositories are available locally, it is normally at `../excalidraw`; verify the actual workspace path and branch instead of assuming it.
200+
201+
- The Excalidraw repository uses Yarn and builds the consumer-specific payload from `packages/excalidraw` with `yarn build:obsidian`.
202+
- That build emits four files under `packages/excalidraw/dist/obsidian/`: production and development JavaScript plus production and development CSS.
203+
- This plugin consumes the same four paths from `node_modules/@zsviczian/excalidraw/dist/obsidian/` in `rollup.config.mjs`.
204+
- For a temporary unpublished integration test, build the sibling package and copy only those four generated files into the installed package under `node_modules`. Do not change `package.json` or `package-lock.json` to a local `file:` dependency merely for this handoff. A later `npm install` restores the published package.
205+
- For the durable handoff, publish a new `@zsviczian/excalidraw` version, update this repository's dependency, run `npm install`, and rebuild the plugin.
206+
- Never hand-edit or commit generated `dist/`, `lib/`, or `node_modules` artifacts as source fixes.
207+
- Treat the repositories as separate Git histories. Check branch, status, diff, build, and commit state independently in each one, and do not commit or publish unless explicitly requested.
208+
203209
### Popout Window Support
204210

205211
- `src/core/managers/PackageManager.ts` manages window-scoped React/ReactDOM/Excalidraw packages.
206212
- This is necessary because the plugin must work in Obsidian/Electron popout windows.
207213
- Do not replace this with a naive global singleton approach.
214+
- The runtime is built from official npm package entry points and kept in plugin/package lexical scope. Do not assign React or ReactDOM to `window`; only the documented `window.ExcalidrawLib` compatibility surface remains global.
215+
- Rendering, DOM ownership, events, observers, portals, and React roots must use the owning view window where appropriate.
216+
217+
### Main-Window Persistent Storage
218+
219+
Window ownership for rendering is not the same as ownership for persistent plugin data.
220+
221+
- Existing plugin-level IndexedDB and local-storage data belongs to Obsidian's main application window and must remain shared across normal views and popouts.
222+
- Do not change persistent storage to `view.ownerWindow`, create one database per popout, or infer a storage migration from a rendering bug unless the task explicitly requires that behavior.
223+
- Diagnose persistence and presentation separately. For example, a visible history button conditioned on loaded records proves the load path worked even when a portaled history menu is hidden.
224+
- If a new feature is intentionally view-local, document that exception and test window migration and popout teardown explicitly.
208225

209226
### MathJax Subproject
210227

@@ -244,6 +261,16 @@ These payloads are executed or unpacked at runtime. This is intentional.
244261
- Preserve existing abstractions unless the task clearly requires a redesign.
245262
- Avoid broad refactors unless there is strong evidence they are necessary.
246263

264+
### Incremental Refactoring Protocol
265+
266+
- Use `RefactorPlan.md` as the living architectural record. Update the progress table and append an action-log entry after each completed or reverted checkpoint.
267+
- Make one independently testable behavior change or mechanical extraction at a time. Prefer moving code intact before simplifying it.
268+
- Preserve timers, observers, semaphores, lifecycle ordering, and unpublished-API workarounds unless their purpose has been traced and an equivalent behavior has been verified across affected platforms.
269+
- Do not convert `ExcalidrawView` wholesale into React. It must remain an Obsidian `TextFileView`; React is the child rendering runtime. Extract cohesive view-scoped controllers and components while retaining compatibility delegates on the view.
270+
- For duplicate utilities, compare every implementation and caller before consolidation. Marginal behavior differences must be shown unused or deliberately preserved.
271+
- Do not derive a runtime settings sanitizer from the TypeScript interface. Interfaces do not exist at runtime, settings evolve frequently, and unknown keys may belong to a newer or companion version. Remove obsolete keys only through an explicit, reviewed migration or retirement decision.
272+
- End every checkpoint with risk-based manual test recommendations: identify the highest-probability failure, the affected workflow, and whether main-window, popout, desktop operating systems, and mobile need separate coverage.
273+
247274
## Naming And Placement Conventions
248275

249276
Treat the following as the target convention for all new code and for any future naming-cleanup pass. The current repository contains legacy exceptions. Do not rename files opportunistically inside behavior changes; do naming cleanup in a dedicated, compatibility-aware refactor.
@@ -419,6 +446,10 @@ Validation guidance:
419446
- Run `npm run lib` if you touch the public/library API surface.
420447
- Run `npm run build:mathjax` or `npm run build:all` if you edit `MathjaxToSVG/`.
421448
- Run `npm run madge` after structural import changes or when touching shared architecture.
449+
- When the customized Excalidraw source changes, run its `yarn build:obsidian`, refresh the four local package artifacts, and then run this repository's production and relevant development builds. A plugin build against the old installed artifact does not validate the component change.
450+
- After React/package-loading changes, validate cold startup, plugin reload, the main window, new and restored popouts, and moving a leaf between windows. Confirm that no `window.React` or `window.ReactDOM` global was introduced.
451+
- After Radix/portal changes, validate visibility, positioning, stacking, click-outside, and Escape handling in both the main window and a popout; include mobile when viewport collision behavior can differ.
452+
- Record `dist/main.js` byte size after packaging changes and report remaining headroom under the release limit.
422453
- Prefer targeted diagnostics for the files you touched when repo-wide lint noise obscures signal.
423454
- Prefer `npm run build` plus targeted file diagnostics over raw `tsc --noEmit` as the primary gate. Standalone `tsc` can surface large volumes of dependency-typing noise unrelated to touched files.
424455
- Do not treat `dist/` output edits as source fixes.
@@ -611,7 +642,7 @@ In some cases, `@typescript-eslint/no-explicit-any` or `@typescript-eslint/no-un
611642

612643
- **Provider-specific dynamic payloads**: AI providers, image APIs, and other external services return schemas that vary by provider. Normalizing these requires accepting `any` properties or using type assertions on the `item` parameter to access provider-specific fields.
613644
- Example: `(item: Record<string, any>) => item.image?.url || item.image?.b64_json` normalizes images from different providers into a common schema.
614-
645+
615646
- **Mutation-path type casts**: When updating scene elements or bound references, Excalidraw type definitions may return readonly or union types, but the mutation path requires the mutable variant. The assertion is necessary and doesn't bypass a real type mismatch.
616647
- Example: `sceneElements.find(...) as unknown as Mutable<ExcalidrawElement>` during ID migration where the lookup guarantees the mutable variant exists.
617648

0 commit comments

Comments
 (0)