[WIP] Address critical gaps in test coverage and performance#439
[WIP] Address critical gaps in test coverage and performance#439
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ations for tree-shaking Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…onProvider Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
… retries=0, retry-then-succeed Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ne/src/__tests__/renderer.test.tsx) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…callbacks Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…t.tsx Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
| // ------------------------------------------------------------------------- | ||
| describe('inline layout', () => { | ||
| it('renders the filter form inline by default', () => { | ||
| const { container } = render(<FilterUI schema={makeSchema()} />); |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 days ago
In general, to fix an unused variable warning, either remove the variable declaration or start using the variable meaningfully. Here, the render call is needed, but the container binding is not. The safest, behavior-preserving fix is to call render without destructuring its return value, since the test body uses only screen APIs.
Concretely, in packages/plugin-view/src/__tests__/FilterUI.test.tsx, in the first test inside the "inline layout" describe block (around line 175), replace const { container } = render(<FilterUI schema={makeSchema()} />); with just render(<FilterUI schema={makeSchema()} />);. No additional imports or helper methods are required.
| @@ -172,7 +172,7 @@ | ||
| // ------------------------------------------------------------------------- | ||
| describe('inline layout', () => { | ||
| it('renders the filter form inline by default', () => { | ||
| const { container } = render(<FilterUI schema={makeSchema()} />); | ||
| render(<FilterUI schema={makeSchema()} />); | ||
|
|
||
| // Labels should be visible | ||
| expect(screen.getByText('Status')).toBeInTheDocument(); |
There was a problem hiding this comment.
Pull request overview
This PR addresses critical gaps in test coverage, integrates internationalization support, and implements performance optimizations through tree-shaking. The changes increase test coverage thresholds from lines: 61%→66%, functions: 43%→55%, branches: 40%→50%, statements: 60%→65%, supported by comprehensive test additions across hooks, contexts, and plugin components.
Changes:
- Added comprehensive test coverage for 10+ previously untested components (useExpression, useDiscovery, useActionRunner, ActionContext, i18n provider, LazyPluginLoader, timeline, SortUI, FilterUI, VirtualGrid)
- Integrated @object-ui/i18n into @object-ui/react, re-exporting the full i18n API for built-in internationalization support
- Added
sideEffects: falseto package.json files to enable tree-shaking in bundlers, reducing bundle size
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.mts | Increased coverage thresholds to reflect improved test coverage |
| pnpm-lock.yaml | Updated lock file with i18n dependency resolution |
| packages/types/package.json | Added sideEffects: false for tree-shaking |
| packages/core/package.json | Added sideEffects: false for tree-shaking |
| packages/i18n/package.json | Added sideEffects: false for tree-shaking |
| packages/layout/package.json | sideEffects: false despite module-level registration side effects |
| packages/react/package.json | Added @object-ui/i18n workspace dependency |
| packages/react/src/index.ts | Re-exported full i18n API from @object-ui/i18n |
| packages/react/src/hooks/tests/useExpression.test.ts | New comprehensive tests for useExpression and useCondition hooks |
| packages/react/src/hooks/tests/useDiscovery.test.tsx | New comprehensive tests for useDiscovery hook including error handling and cleanup |
| packages/react/src/hooks/tests/useActionRunner.test.ts | New comprehensive tests for useActionRunner hook covering all execution paths |
| packages/react/src/context/tests/ActionContext.test.tsx | New comprehensive tests for ActionProvider, useAction, and useHasActionProvider |
| packages/react/src/tests/LazyPluginLoader.test.tsx | Added tests for preloadPlugin, errorFallback, and retry mechanisms |
| packages/i18n/src/tests/provider.test.tsx | New comprehensive tests for i18n provider and hooks |
| packages/plugin-view/src/tests/SortUI.test.tsx | New comprehensive tests covering all SortUI variants and behaviors |
| packages/plugin-view/src/tests/FilterUI.test.tsx | New comprehensive tests covering all FilterUI layouts and filter types |
| packages/plugin-timeline/src/tests/renderer.test.tsx | New comprehensive tests for timeline renderer (vertical, horizontal, gantt) |
| packages/plugin-grid/src/tests/VirtualGrid.test.tsx | New comprehensive tests for VirtualGrid with proper virtualizer mocking |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| "name": "@object-ui/layout", | ||
| "version": "0.5.0", | ||
| "type": "module", | ||
| "sideEffects": false, |
There was a problem hiding this comment.
Setting sideEffects: false is incorrect for this package. The entry point src/index.ts has module-level side effects (calls registerLayout() at lines 53-57 to register components in ComponentRegistry). If bundlers tree-shake this module due to sideEffects: false, the layout components will not be registered and will fail at runtime. Either remove this field or set it to true, or move the registration call out of the module-level scope.
| "sideEffects": false, | |
| "sideEffects": true, |
| // ------------------------------------------------------------------------- | ||
| describe('inline layout', () => { | ||
| it('renders the filter form inline by default', () => { | ||
| const { container } = render(<FilterUI schema={makeSchema()} />); |
There was a problem hiding this comment.
Unused variable container.
| const { container } = render(<FilterUI schema={makeSchema()} />); | |
| render(<FilterUI schema={makeSchema()} />); |
import { I18nProvider, useObjectTranslation } from '@object-ui/react'sideEffects: falseto @object-ui/types, @object-ui/core, @object-ui/i18n, @object-ui/layout💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.