Document module script entries (scriptType) and the src/ tree#17
Conversation
The stems plugin's R1 pilot split its client code from one monolithic
screen.js into an ES-module tree: plugin.json declares
`scriptType: "module"`, screen.js is a one-line `import './src/main.js'`,
and the code lives under src/. The current feedback Host supports this —
it injects `<script type="module">` for `scriptType:"module"` and serves
the src/ tree at /api/plugins/{id}/src/{path} — but the spec did not
describe it, and best-practices actively contradicted it ("screen.js runs
as a classic script", "the plugin root is not a servable directory").
- spec §3/§4.1/§4.3, new §6.8, schema: document the optional `scriptType`
and `minHost` keys and the served src/ module tree. Purely additive.
- best-practices rules 27-29 rewritten: module entry is the recommended
split; classic bundle / window-share kept as the older-Host fallback.
- CHANGELOG: Unreleased entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates the schema, spec, best-practices guide, and changelog to define ChangesscriptType and module splitting documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/best-practices.md`:
- Around line 455-460: The split-runtime guidance in best-practices.md hardcodes
the plugin asset route, which conflicts with Host-assigned asset URLs. Update
the “Split at runtime” section to tell authors to use the Host-provided asset
URL/base instead of `/api/plugins/<id>/assets/…`, and keep the checklist bullet
aligned; the relevant guidance is in the split-runtime asset-loading note and
the classic-script state-sharing example.
- Around line 422-429: The fenced examples in this best-practices section are
unlabeled, which triggers MD040 and makes them harder to read. Update the code
fences in the tree example and the runtime snippet to use the appropriate
language tags, using text for the directory tree and js for the JavaScript
example. Apply the same fix to the matching fenced block referenced later in the
document so all examples use explicit labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bf35df4-9fc0-4e9d-9fb9-55b6cd24eb39
📒 Files selected for processing (4)
CHANGELOG.mdschemas/plugin.schema.jsonspec/best-practices.mdspec/plugin-spec-v1.md
- Label the src/-tree fence as `text` (MD040). - Stop hardcoding the /api/plugins/<id>/assets route in the classic-split guidance and checklist; derive the asset base from the script's own served URL (document.currentScript) instead, per normative §6.7 which says asset URLs are Host-assigned. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Kris Anderson <topkoa@gmail.com>
Summary
The stems plugin's R1 pilot (its PRs #23–#36) split its client code from one monolithic
screen.jsinto an ES-module tree:plugin.jsondeclares"scriptType": "module",screen.jsis a one-lineimport './src/main.js';, and the code lives undersrc/sharing state viaimport/export.The current feedBack Host already supports this — it injects
<script type="module">whenscriptTypeis"module", and serves the module tree at/api/plugins/{id}/src/{path}— but the spec did not describe it, and best-practices actively contradicted it ("screen.jsruns as a classic script", "the plugin root is not a servable directory"). This PR brings the spec up to date.Changes (purely additive — every existing manifest still validates)
spec/plugin-spec-v1.md— §3 anatomy shows the optionalsrc/tree; §4.1/§4.3 document the newscriptTypeandminHostkeys; §6.1 notes classic-vs-module loading; new §6.8 "Splitting client code across modules."schemas/plugin.schema.json— addedscriptTypeandminHostproperties.spec/best-practices.md— rewrote rules 27–29 (and the checklist): an ES-module entry (scriptType: "module"+src/) is now the recommended way to split client code; the classic single-screen.jsbundle /window-shared runtime split is kept as the fallback for older Hosts. Corrects text that was flatly wrong oncescriptType: "module"is set.CHANGELOG.md—[Unreleased]entry.Per spec §9 this is additive (a new OPTIONAL manifest key an older Host ignores), so no manifest-major bump.
Verification
schemas/plugin.schema.jsonis valid JSON;tools/validate.pypasses both examples and the real stemsmainmanifest.11 passed.🤖 Generated with Claude Code
Summary by CodeRabbit
scriptTypemanifest setting, enablingsrc/-based code splitting.minHostguidance for module-dependent plugins to prevent older hosts from loading module entries incorrectly.scriptTypeandminHost, and clarified thescriptfield’s meaning.