best-practices: expand id/naming rule (collisions + namespacing) - #6
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR refines the non-normative best-practices guidance around plugin id selection, focusing on real-world collision behavior and the practical namespacing implications across Host-derived surfaces.
Changes:
- Expanded best-practices rule 2 to document
idscope/derivations, collision precedence, reserved ids, and shared-namespace namespacing guidance. - Added matching items to the pre-publish checklist.
- Documented the update in the
[Unreleased]changelog entry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| spec/best-practices.md | Expands rule 2 (id/naming) and updates the publish checklist accordingly. |
| CHANGELOG.md | Adds an [Unreleased] entry summarizing the best-practices update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e792ad2 to
b2040e8
Compare
…tchas Rule 2 was a short "treat the id as forever". Expand it, ground-truthed against the plugin loader, into the real id/naming pitfalls: - How far the id reaches: routes module (plugin_<id>_routes), screen container (plugin-<id>), viz global (window.feedBackViz_<id>), diagnostics path, and the localStorage prefix — so a collision or rename ripples everywhere. - Folder name MUST equal id exactly incl. case (the #1 "won't load" cause), and the charset matters because id becomes a Python module name + DOM/JS identifier — uppercase/dots/spaces break discovery or module loading; the validator rejects them. - Collision resolution: a bundled id ALWAYS wins (a user plugin reusing it is silently ignored, kept only as a fallback); between two non-bundled plugins the first discovered wins. Check your id isn't a bundled one. - Reserved ids: capability_inspector and app_tour_* are always-enabled — don't collide with them. - Namespace shared-space names (localStorage, window globals, routes, CSS) by id, since all plugins share one window and document. Cross-link spec §4.2 / §5.2 for the normative rules. Add matching checklist items. Numbering unchanged (enriched in place). Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: K. O. A. <topkoa@gmail.com>
- Rule 2: drop 'snake_case is the house style' (examples use kebab-case; both '-' and '_' are valid and used). Say lowercase with -/_ separators. - Checklist: limit the new namespacing item to localStorage/window globals; routes and CSS are already covered by the later item. Signed-off-by: K. O. A. <topkoa@gmail.com>
3897901 to
675d076
Compare
Summary
Refines the
id/naming best-practice (rule 2) with the real collision and namespacing gotchas, ground-truthed against the plugin loader — the "what should I name it and what will bite me?" guidance.What it now covers
idreaches. It's interpolated into the routes module (plugin_<id>_routes), the screen container (plugin-<id>), a visualization's factory global (window.feedBackViz_<id>), the diagnostics path (plugins/<id>/), and thelocalStorageprefix — so a collision or rename ripples through all of them.idexactly including case (the Scaffold the feedBack plugin specification (v0.1.0 draft) #1 "why won't it load?"), and^[a-z0-9][a-z0-9_-]*$isn't cosmetic — theidbecomes a Python module name and DOM/JS identifiers, so uppercase/dots/spaces break discovery or module loading. The validator rejects them.idalways wins — a user plugin reusing a bundledidis silently ignored (kept only as a fallback). Between two non-bundled plugins, the first discovered wins and the other silently doesn't load.capability_inspectorand theapp_tour_*prefix are always-enabled (can't be disabled) — don't collide with them.iddoesn't do for you. All plugins share onewindowand one document, so prefixlocalStoragekeys,windowglobals, routes, and CSS byid— two plugins writingwindow.stateorlocalStorage["theme"]clobber each other silently.Cross-links the normative rules in spec §4.2 (
id) and §5.2 (directory-name rule). Added matching checklist items.Scope & stacking
Docs only (
spec/best-practices.md+CHANGELOG.md). No version bump; rule enriched in place so numbering is unchanged (1–25). The normative id rules already live in the spec (§4.2 charset/stability, §5.3 bundled-wins precedence) — this is the actionable advice layer, so no spec change needed.Stacked on #5 (base
docs/best-practices-visualizations). Full stack: #2 → #4 → #3 → #5 → #6.mkdocs build --strictandcheck_versions.pypass.🤖 Generated with Claude Code