Repository navigation
fix: prevent pnpm dev startup failures (getService crash, host config mis-wrap, ESM exports) #905
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
Changes from all commits
08bd82e
c3aeae9
0aad36a
2600467
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. | ||
|
|
||
| /** | ||
| * Detect whether a loaded config is a host/aggregator configuration. | ||
| * | ||
| * A host config already contains a `plugins` array with instantiated Plugin | ||
| * objects (i.e. objects that have an `init` method). Such configs must NOT | ||
| * be wrapped again by AppPlugin in the CLI, as that would cause duplicate | ||
| * plugin registration and startup failures. | ||
| */ | ||
| export function isHostConfig(config: any): boolean { | ||
| return ( | ||
| Array.isArray(config.plugins) && | ||
| config.plugins.some((p: any) => typeof p?.init === 'function') | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { isHostConfig } from '../src/utils/plugin-detection'; | ||
|
|
||
| /** | ||
| * Tests for the host config detection logic used in serve.ts | ||
| * | ||
| * When the root config is a host/aggregator (i.e. its `plugins` array | ||
| * contains already-instantiated Plugin objects), the CLI must NOT wrap | ||
| * it again with AppPlugin, as that would cause duplicate registration | ||
| * and a `plugin.app.<id> failed to start` error. | ||
| */ | ||
| describe('Host config detection', () => { | ||
|
|
||
| it('should detect host config with instantiated plugins', () => { | ||
| const config = { | ||
| manifest: { id: 'dev-workspace', name: 'dev_workspace' }, | ||
| plugins: [ | ||
| { name: 'objectql', init: async () => {}, start: async () => {} }, | ||
| { name: 'driver', init: async () => {}, start: async () => {} }, | ||
| ], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(true); | ||
| }); | ||
|
|
||
| it('should NOT detect pure app bundle config (no plugins)', () => { | ||
| const config = { | ||
| manifest: { id: 'my-app', name: 'my_app' }, | ||
| objects: [{ name: 'task', fields: [] }], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(false); | ||
| }); | ||
|
|
||
| it('should NOT detect config with empty plugins array', () => { | ||
| const config = { | ||
| manifest: { id: 'my-app', name: 'my_app' }, | ||
| objects: [{ name: 'task', fields: [] }], | ||
| plugins: [], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(false); | ||
| }); | ||
|
|
||
| it('should NOT detect config with string plugin references', () => { | ||
| const config = { | ||
| manifest: { id: 'my-app', name: 'my_app' }, | ||
| plugins: ['@objectstack/plugin-auth', '@objectstack/objectql'], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(false); | ||
| }); | ||
|
|
||
| it('should NOT detect config with plain object plugins (no init method)', () => { | ||
| const config = { | ||
| manifest: { id: 'my-app', name: 'my_app' }, | ||
| plugins: [ | ||
| { name: 'some-plugin', version: '1.0.0' }, | ||
| ], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(false); | ||
| }); | ||
|
|
||
| it('should detect if at least one plugin has init method', () => { | ||
| const config = { | ||
| manifest: { id: 'dev-workspace' }, | ||
| plugins: [ | ||
| { name: 'plain-bundle', version: '1.0.0' }, | ||
| { name: 'real-plugin', init: async () => {}, start: async () => {} }, | ||
| ], | ||
| }; | ||
| expect(isHostConfig(config)).toBe(true); | ||
| }); | ||
|
|
||
| it('should handle config without plugins property', () => { | ||
| const config = { | ||
| manifest: { id: 'my-app', name: 'my_app' }, | ||
| }; | ||
| expect(isHostConfig(config)).toBe(false); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,9 +60,15 @@ export class AppPlugin implements Plugin { | |||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Execute Runtime Step | ||||||||||||||||||||||||||
| // Retrieve ObjectQL engine from services | ||||||||||||||||||||||||||
| // We cast to any/ObjectQL because ctx.getService returns unknown | ||||||||||||||||||||||||||
| const ql = ctx.getService('objectql') as any; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // ctx.getService throws when a service is not registered, so we | ||||||||||||||||||||||||||
| // must use try/catch instead of a null-check. | ||||||||||||||||||||||||||
| let ql: any; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| ql = ctx.getService('objectql'); | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| // Service not registered — handled below | ||||||||||||||||||||||||||
|
Comment on lines
+68
to
+69
|
||||||||||||||||||||||||||
| } catch { | |
| // Service not registered — handled below | |
| } catch (error: unknown) { | |
| const message = error instanceof Error ? error.message : String(error); | |
| if (message !== "[Kernel] Service 'objectql' not found") { | |
| // Re-throw non-"not found" errors so real startup failures are not masked | |
| throw error; | |
| } | |
| // "[Kernel] Service 'objectql' not found" — handled below via !ql check |
Copilot
AI
Mar 11, 2026
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.
Same issue as objectql: this try/catch around ctx.getService('i18n') currently swallows all errors, including async-service errors or factory failures that the kernel intentionally rethrows. Narrow the catch to only ignore the explicit "[Kernel] Service 'i18n' not found" case (or equivalent not-found sentinel) and rethrow other errors so real i18n startup failures aren’t silently skipped.
| } catch { | |
| // Service not registered — handled below | |
| } catch (err: unknown) { | |
| if ( | |
| err instanceof Error && | |
| err.message && | |
| err.message.includes("Service 'i18n' not found") | |
| ) { | |
| // Service not registered — handled below | |
| } else { | |
| throw err; | |
| } |
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.
The new conditional exports map has
typesafterimport/require. In this repo’s packages,typesis consistently listed first inexports["."](e.g. @objectstack/core, @objectstack/runtime), which avoids tooling picking the runtime entry when resolving typings. Reorder the keys to puttypesfirst to match existing package conventions.