Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export default defineConfig({
globals: true,
environment: 'happy-dom',
setupFiles: ['../../vitest.setup.ts'],
passWithNoTests: true,
// Ensure dependencies are resolved properly for tests
deps: {
inline: ['@object-ui/core', '@object-ui/react'],
Expand Down
36 changes: 20 additions & 16 deletions packages/core/src/registry/Registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ export class Registry<T = any> {
register(type: string, component: ComponentRenderer<T>, meta?: ComponentMeta) {
const fullType = meta?.namespace ? `${meta.namespace}:${type}` : type;

// Warn if registering without namespace (deprecated pattern)
if (!meta?.namespace) {
console.warn(
`Registering component "${type}" without a namespace is deprecated. ` +
`Please provide a namespace in the meta parameter.`
);
}
Comment on lines +75 to +81
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation warning triggers for both undefined namespace and empty string namespace. Since empty string is explicitly handled as "no namespace" (line 73), it may be intentional to allow empty string without warning. Consider whether the warning should check for explicit empty string and handle it differently, or if empty strings should also be deprecated. The current behavior may produce unexpected warnings for users who explicitly pass { namespace: '' }.

Copilot uses AI. Check for mistakes.
Comment on lines +75 to +81
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation warning for non-namespaced registration will trigger for every component registration in the codebase that doesn't provide a namespace. Based on a search of the codebase, there are 100+ component registrations without a namespace (in packages/components, packages/layout, and various plugins). This will result in console spam with hundreds of deprecation warnings during normal application startup. Before adding this deprecation warning, either: (1) migrate all existing registrations to use namespaces first, or (2) make this warning opt-in via a flag, or (3) throttle/deduplicate the warnings, or (4) remove the deprecation warning until the codebase is ready for the migration.

Copilot uses AI. Check for mistakes.

if (this.components.has(fullType)) {
// console.warn(`Component type "${fullType}" is already registered. Overwriting.`);
}
Expand All @@ -84,7 +92,9 @@ export class Registry<T = any> {

// Also register without namespace for backward compatibility
// This allows "button" to work even when registered as "ui:button"
if (meta?.namespace && !this.components.has(type)) {
// Note: If multiple namespaced components share the same short name,
// the last registration wins for non-namespaced lookups
if (meta?.namespace) {
Comment on lines +95 to +97
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the !this.components.has(type) condition changes the backward compatibility behavior. Previously, if a component was registered without a namespace, and then another component with the same name was registered with a namespace, the non-namespaced entry would be preserved. Now, every namespaced registration overwrites the non-namespaced entry for that type name. While this is acknowledged in the comment at lines 95-96, this is a subtle behavioral change that could affect existing code. Consider whether this change is intentional, and if so, ensure it's clearly documented in the PR description and migration guide.

Suggested change
// Note: If multiple namespaced components share the same short name,
// the last registration wins for non-namespaced lookups
if (meta?.namespace) {
// Preserve existing non-namespaced registrations for backward compatibility.
// Only create the short-name alias if it does not already exist.
if (meta?.namespace && !this.components.has(type)) {

Copilot uses AI. Check for mistakes.
this.components.set(type, {
type: fullType, // Keep reference to namespaced type
component,
Expand Down Expand Up @@ -113,16 +123,13 @@ export class Registry<T = any> {
* registry.get('button', 'ui') // Tries 'ui:button' first, then 'button'
*/
get(type: string, namespace?: string): ComponentRenderer<T> | undefined {
// Try namespaced lookup first if namespace provided
// If namespace is explicitly provided, ONLY look in that namespace (no fallback)
if (namespace) {
const namespacedType = `${namespace}:${type}`;
const namespacedComponent = this.components.get(namespacedType);
if (namespacedComponent) {
return namespacedComponent.component;
}
return this.components.get(namespacedType)?.component;
}

// Fallback to direct type lookup
// When no namespace provided, use backward compatibility lookup
return this.components.get(type)?.component;
}

Expand All @@ -134,16 +141,13 @@ export class Registry<T = any> {
* @returns Component configuration or undefined
*/
getConfig(type: string, namespace?: string): ComponentConfig<T> | undefined {
// Try namespaced lookup first if namespace provided
// If namespace is explicitly provided, ONLY look in that namespace (no fallback)
if (namespace) {
const namespacedType = `${namespace}:${type}`;
const namespacedConfig = this.components.get(namespacedType);
if (namespacedConfig) {
return namespacedConfig;
}
return this.components.get(namespacedType);
}

// Fallback to direct type lookup
// When no namespace provided, use backward compatibility lookup
return this.components.get(type);
}

Expand All @@ -155,12 +159,12 @@ export class Registry<T = any> {
* @returns True if component is registered
*/
has(type: string, namespace?: string): boolean {
// If namespace is explicitly provided, ONLY look in that namespace (no fallback)
if (namespace) {
const namespacedType = `${namespace}:${type}`;
if (this.components.has(namespacedType)) {
return true;
}
return this.components.has(namespacedType);
}
// When no namespace provided, use backward compatibility lookup
return this.components.has(type);
}

Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/registry/__tests__/PluginSystem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ describe('PluginSystem', () => {
}
};

await pluginSystem.loadPlugin(plugin, registry);
// Use legacy mode (useScope: false) to test direct registry access
await pluginSystem.loadPlugin(plugin, registry, false);

expect(pluginSystem.isLoaded('test-plugin')).toBe(true);
expect(pluginSystem.getLoadedPlugins()).toContain('test-plugin');
Expand Down Expand Up @@ -201,7 +202,8 @@ describe('PluginSystem', () => {
register: registerFn
};

await pluginSystem.loadPlugin(plugin, registry);
// Use legacy mode (useScope: false) to verify the raw Registry is passed
await pluginSystem.loadPlugin(plugin, registry, false);

expect(registerFn).toHaveBeenCalledWith(registry);
expect(registerFn).toHaveBeenCalledTimes(1);
Expand Down
1 change: 1 addition & 0 deletions packages/fields/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ export default defineConfig({
globals: true,
environment: 'happy-dom',
setupFiles: ['../../vitest.setup.ts'],
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/layout/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,7 @@ export default defineConfig({
],
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-aggrid/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-calendar/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-charts/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-chatbot/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-dashboard/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-editor/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-form/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-gantt/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-grid/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-kanban/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-map/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-markdown/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-timeline/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
3 changes: 3 additions & 0 deletions packages/plugin-view/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,7 @@ export default defineConfig({
},
},
},
test: {
passWithNoTests: true,
},
});
1 change: 1 addition & 0 deletions vitest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default defineConfig({
environment: 'happy-dom',
setupFiles: [path.resolve(__dirname, 'vitest.setup.ts')],
exclude: ['**/node_modules/**', '**/dist/**', '**/cypress/**', '**/.{idea,git,cache,output,temp}/**'],
passWithNoTests: true,
coverage: {
provider: 'v8',
reporter: ['text', 'json', 'html'],
Expand Down
Loading