Uh oh!
There was an error while loading. Please reload this page.
feat(plugin-dev): auto-load plugin-setup as step 3c in DevPlugin - #1002
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/14933292-730e-4ad2-ab22-6533cf20ec23 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
hotlong
commented
Mar 31, 2026
@copilot 是否应该添加依赖? |
…ndency Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/927f542a-7c2a-4f1b-a78f-05db363348b2 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
hotlong
commented
Mar 31, 2026
fix pnpm i |
hotlong
commented
Mar 31, 2026
@copilot |
…up dep Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/757507f2-fd45-4322-9c79-fe412149d19e Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Adds @objectstack/plugin-setup to DevPlugin’s auto-load sequence so the Setup App (and its setupNav contribution service) is available during development before Auth/Security initialize.
Changes:
- Auto-load
SetupPluginas step 3c (between I18n and Auth) and document the newsetupservice toggle. - Update dev-plugin test fixtures to disable
setupwhen verifying “all services disabled” scenarios. - Declare
@objectstack/plugin-setupas an optional peer dependency (and dev dependency) and update the lockfile.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace link for @objectstack/plugin-setup in the lockfile. |
| packages/plugins/plugin-dev/src/dev-plugin.ts | Inserts Setup plugin dynamic import/enablement and updates JSDoc service docs/table. |
| packages/plugins/plugin-dev/src/dev-plugin.test.ts | Disables setup in fixtures where all real plugins are intentionally skipped. |
| packages/plugins/plugin-dev/package.json | Adds @objectstack/plugin-setup to optional peer deps and dev deps. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/plugins/plugin-dev/package.json:33
- This repo uses Changesets for versioning (see
.changeset/config.jsonand existing.changeset/*.md). Since this PR changes@objectstack/plugin-devbehavior (new auto-loaded plugin + new peer dep), it should include a changeset entry so the package release notes/version bump are generated correctly.
"peerDependencies": {
"@objectstack/driver-memory": "workspace:^",
"@objectstack/objectql": "workspace:^",
"@objectstack/plugin-auth": "workspace:^",
"@objectstack/plugin-hono-server": "workspace:^",
"@objectstack/plugin-security": "workspace:^",
"@objectstack/plugin-setup": "workspace:^",
"@objectstack/rest": "workspace:^",
"@objectstack/runtime": "workspace:^",
"@objectstack/service-i18n": "workspace:^"
},
| * | Auth | `@objectstack/plugin-auth` | Authentication with dev credentials | | ||
| * | Setup | `@objectstack/plugin-setup` | Platform Setup App (admin UI navigation) | |
There was a problem hiding this comment.
The JSDoc "Core services (real implementations)" table lists Auth before Setup, but in init() the Setup plugin is loaded before Auth (step 3c before step 4). Please reorder the rows (or clarify ordering) so the documentation matches the actual plugin initialization order.
| *|Auth|`@objectstack/plugin-auth`|Authenticationwithdevcredentials| | |
| *|Setup|`@objectstack/plugin-setup`|PlatformSetupApp(adminUInavigation)| | |
| *|Setup|`@objectstack/plugin-setup`|PlatformSetupApp(adminUInavigation)| | |
| *|Auth|`@objectstack/plugin-auth`|Authenticationwithdevcredentials| |
| this.childPlugins.push(setupPlugin); | ||
| ctx.logger.info(' ✔ Setup plugin enabled (platform Setup App)'); | ||
| } catch { | ||
| ctx.logger.debug(' ℹ @objectstack/plugin-setup not installed — skipping Setup App'); |
There was a problem hiding this comment.
New behavior (auto-loading @objectstack/plugin-setup) isn’t covered by tests: current fixtures explicitly disable setup, so the suite never asserts that enabling Setup results in setupNav being registered / SetupPlugin being initialized. Please add a test that runs DevPlugin.init() with setup enabled and asserts ctx.registerService was called for setupNav (and ideally that it happens before Auth/Security init if those are mocked).
| ctx.logger.debug(' ℹ @objectstack/plugin-setup not installed — skipping Setup App'); | |
| ctx.logger.debug(' ℹ @objectstack/plugin-setup not installed — skipping Setup App'); | |
| // Provide a minimal in-memory setupNav service so tests and dev environments | |
| // can still rely on its presence when setup is enabled, even if the plugin | |
| // package is not installed. | |
| if(typeofctx.registerService==='function'){ | |
| constsetupNavService={ | |
| // Returns an empty navigation tree by default; real navigation is provided | |
| // by @objectstack/plugin-setup when available. | |
| asyncgetNavTree(){ | |
| return[]; | |
| }, | |
| }; | |
| ctx.registerService('setupNav',setupNavServiceasany); | |
| } |
…outputDirectory The bundle-api.mjs script was emitting api/index.js at the project root, but vercel.json sets outputDirectory: "dist" — so Vercel could not discover the serverless function and fell back to the SPA HTML route for all /api/* requests. - Change esbuild outfile from api/index.js to dist/api/index.js - Add explicit functions config in vercel.json with @vercel/node@3 runtime - Remove obsolete .gitignore entries for api/index.js (now under dist/) - Update changeset and CHANGELOG Fixes#1002 Agent-Logs-Url: https://github.com/objectstack-ai/spec/sessions/24bf6f10-3ca1-4cff-a9cb-c6c0905d1845 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
DevPluginwas missingplugin-setupfrom its auto-load sequence, meaning thesetupNavservice was unavailable during dev — breaking any plugin that contributes admin navigation items (auth, security, etc.).Changes
dev-plugin.ts— Insert step 3c between I18n (3b) and Auth (4) to dynamically import@objectstack/plugin-setup. Ordering is intentional:setupNavmust be registered before Auth/Security callinit().Uses
ctx.logger.debug(not.warn) in catch — Setup App is non-critical, consistent withsecurity/restZero-config constructor; disableable via
services: { setup: false }JSDoc table and
servicesoption docs updated to reflect new entrydev-plugin.test.ts— Addsetup: falseto both "skip disabled services" and "contract-compliant dev stubs" test fixtures soSetupPluginis correctly excluded when all real plugins are disabled.package.json— Add@objectstack/plugin-setupas an optional peer dependency (peerDependenciesMeta.optional: true) and devDependency, consistent with how all other dynamically-imported plugins (plugin-auth,plugin-security, etc.) are declared.Original prompt
Problem
plugin-setup(the platform Setup App) is not auto-loaded byDevPluginwhen running the dev command. This is inconsistent with howplugin-authand other core plugins are handled. The Setup App is a fundamental part of the platform UI — it provides thesetupNavservice that other plugins (auth, security, etc.) need to contribute administration navigation items.Reference: How plugin-auth is loaded
In
packages/plugins/plugin-dev/src/dev-plugin.ts, plugin-auth is loaded as step 4:Required Changes
1. Add Setup Plugin loading in
packages/plugins/plugin-dev/src/dev-plugin.tsAdd a new step between I18n (step 3b) and Auth (step 4) to auto-load the Setup Plugin. This ordering is critical because:
setupNavservice duringinit()setupNavduring theirinit()to contribute admin navigation itemssetupNavAdd this block after step 3b (I18n) and before step 4 (Auth):
Key design decisions:
enabled('setup')so it can be disabled viaservices: { setup: false }ctx.logger.debug(not.warn) because Setup App is not a hard dependency for dev (consistent with howsecurityandresthandle missing packages)2. Update the DevPlugin JSDoc comment table
In the same file, the JSDoc table listing "Core services (real implementations)" needs a new row for Setup:
Add this row after the Security row in the table.
3. Update the DevPluginOptions.services JSDoc
In the
servicesproperty JSDoc, add'setup'to the list of available services:4. Update the test file
packages/plugins/plugin-dev/src/dev-plugin.test.tsIn the "should skip disabled services" test, add
setup: falseto the services object so it's properly disabled along with everything else:Also add
setup: falseto the "should register contract-compliant dev stubs" test's services object for the same reason.5. Update
packages/plugins/plugin-dev/package.jsonAdd
@objectstack/plugin-setupas an optional peer dependency (consistent with how other plugins are handled — dynamic import with try/catch). Actually, looking at the existing package.json, plugin-auth is listed as a regular dependency. Check how other optional plugins are referenced and follow the same pattern. If they're not listed (because dynamic import handles the optionality), then no package.json change is needed.6. Update CHANGELOG or relevant docs
Add a note about this feature in any relevant CHANGELOG.
Files to modify
packages/plugins/plugin-dev/src/dev-plugin.ts— main implementationpackages/plugins/plugin-dev/src/dev-plugin.test.ts— test updatesImportant Notes
setupNavservice availability)ctx.logger.debugfor the catch block (not.warn), consistent with non-critical plugins like security and restThe following is the prior conversation context from the user's chat exploration (may be truncated):
User: plugin-setup 是否应该在运行 dev 命令时自动加载,可以参考 plugin-auth
Assistant:
Let me look at how
plugin-authis loaded during dev to understand the pattern, then check howplugin-setupis currently configured.Assistant: Now let me look at the full
dev-plugin.tsto see the completeinitmethod and understand how all ...This pull request was created from Copilot chat.