chore: review module dependencies warns#3194
Conversation
ommeirelles
requested review from
hellofanny and
lariciamota
and removed request for
a team
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (4)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
WalkthroughThis pull request migrates testing from Jest to Vitest across the components and sdk packages, adds Vitest configs and setup files, removes per-package Jest configs and setup, updates TypeScript test types, upgrades Storybook to v10 (removing an addon), and bumps core dev tooling and packageManager. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/components/src/molecules/Dropdown/index.ts`:
- Around line 12-31: The pull makes all Dropdown exports lazy which breaks
consumers that render Dropdown without a Suspense boundary (e.g.,
MyAccountOrderActions); restore a synchronous default export for Dropdown and
the other components (DropdownButton, DropdownMenu, DropdownItem) and add
explicit lazy variants (e.g., DropdownLazy, DropdownButtonLazy,
DropdownMenuLazy, DropdownItemLazy) that use lazy(...) so downstream code that
doesn’t use Suspense keeps working while consumers that want code-splitting can
opt into the *_Lazy exports; update the exports in this module accordingly and
keep existing import paths/names for synchronous usage.
In `@packages/components/test/molecules/Card/Card.test.tsx`:
- Line 12: mockIconAction is declared once at describe scope and not reset
between tests, causing state leakage; fix by either moving its creation into a
beforeEach (e.g., set mockIconAction = vi.fn() inside beforeEach) or calling
mockIconAction.mockReset()/mockClear() in beforeEach (or use vi.clearAllMocks())
so each test gets a fresh mock; update tests that reference mockIconAction
accordingly (e.g., the "executes the icon action" and accessibility tests) to
rely on the reset mock.
In `@packages/components/test/molecules/CheckboxField/CheckboxField.test.tsx`:
- Around line 73-74: The TypeScript suppression hides that the matcher typing
for toHaveNoViolations (used with axe in CheckboxField.test.tsx) is missing from
vitest-axe; fix by either upgrading vitest-axe to a version that ships proper
type definitions or add a small declaration-merge file (e.g., a global .d.ts in
your test types) that augments Vitest's expect/Assertion interface with the
toHaveNoViolations matcher signature; then remove the // `@ts-expect-error` and
run typechecks to ensure axe(container) and expect(...).toHaveNoViolations()
compile cleanly.
In `@packages/core/package.json`:
- Line 93: Root package.json is missing the `@lhci/cli` devDependency which causes
the dependency checker in packages/cli/src/utils/generate.ts to warn; add
"@lhci/cli": "^0.15.1" to the root devDependencies block in package.json to
match the version used in core, then run install to update the lockfile so the
checker no longer emits warnings.
In `@packages/sdk/test/store/optimistic.test.ts`:
- Around line 10-13: The test's mocked validator onValidate is leaving its
parameter x implicitly any; update the mock so the validator is explicitly typed
for numbers (e.g., annotate the mock or the variable as (x: number) =>
Promise<number | null>) so it matches the expected signature used by getStore(0,
onValidate) and satisfies strict typing.
🧹 Nitpick comments (6)
packages/sdk/test/search/usePagination.test.tsx (2)
8-28: Consider removing unnecessaryasynckeywords.Both test functions are marked
asyncbut contain noawaitstatements. This is harmless but adds noise.♻️ Optional cleanup
-test('usePagination: paginates forwards', async () => { +test('usePagination: paginates forwards', () => {-test('usePagination: paginates backwards', async () => { +test('usePagination: paginates backwards', () => {Also applies to: 30-54
11-11: TypeScript safety:PropsWithChildren<any>could be tightened.Using
anyweakens type checking. Since wrapper only needschildren, consider usingunknownor an empty object type.♻️ Stricter typing
- wrapper: ({ children }: PropsWithChildren<any>) => ( + wrapper: ({ children }: PropsWithChildren<unknown>) => (Also applies to: 36-36
packages/components/src/molecules/Breadcrumb/BreadcrumbBase.tsx (1)
141-163: Add Suspense fallback for lazy-loaded Dropdown.
Lazy components without a fallback can cause layout shift. Provide a lightweight placeholder to maintain layout stability while code chunks load.♻️ Suggested fallback
- {collapseBreadcrumb && ( - <Suspense> + {collapseBreadcrumb && ( + <Suspense + fallback={ + <span + aria-hidden="true" + data-fs-breadcrumb-dropdown-fallback + /> + } + >packages/components/vitest.setup.ts (1)
1-6: Redundant axe matcher extension.Lines 2-3 manually extend
expectwithvitest-axe/matchers, but line 6 importsvitest-axe/extend-expectwhich does the same thing. Pick one approach.♻️ Simplified setup using side-effect imports only
-import { expect } from 'vitest' -import * as matchers from 'vitest-axe/matchers' -expect.extend(matchers) - import '@testing-library/jest-dom/vitest' import 'vitest-axe/extend-expect'packages/components/test/molecules/Card/Card.test.tsx (1)
52-53: Type suppression forvitest-axenoted.The
@ts-expect-errorindicates a type mismatch betweenvitest-axeand the extended matchers. This is a known issue with early versions ofvitest-axe. Consider tracking this as tech debt to remove once types are fixed upstream or by adding proper type augmentation in your setup file.packages/components/package.json (1)
41-51:@testing-library/jest-domand@vitejs/plugin-reactcan remain as hardcoded versions;vitest-axewarrants a quick accessibility verification.These packages are only used in
packages/components, so adding them to the pnpm catalog isn't necessary for monorepo consistency. However, verify thatvitest-axe@^0.1.0meets your accessibility testing requirements—it's a pre-1.0 release and may lack stability or features you need.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/package.json`:
- Line 93: Upgrade to `@lhci/cli`@0.15.1 changed Lighthouse behavior causing
assertion failures; run the Lighthouse CI pipeline against your target pages and
update the thresholds in lighthouserc.js accordingly—specifically adjust
categories.best-practices.minScore and any resource-summary threshold entries to
match the new Lighthouse 12.6.1 outputs, re-run until the assertions pass, and
commit the new baseline values.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/storybook/.storybook/main.ts`:
- Around line 3-14: The getAbsolutePath function uses path.join(join(value,
'package.json')) which produces Windows backslashes and breaks
import.meta.resolve; replace join(value, 'package.json') with a POSIX-safe
template string like `${value}/package.json` inside import.meta.resolve in the
getAbsolutePath function so the ESM specifier remains forward-slash-separated on
all platforms and ensure the surrounding parentheses are balanced around
import.meta.resolve and fileURLToPath.
2 participants
Updated dependencies:
"@testing-library/jest-dom"
"@lhci/cli": "^0.15.1",
"@graphql-codegen/cli": 6.1.1 - Updated to V6 as it was triggering warns in respect to their dependencies
[REMOVED] @types/cypress - Its not necessary on our current version of cypress anymore
[REMOVED] "@storybook/addon-interactions": "^8.6.14" - Is part of core storybook package now
[REMOVED] "@storybook/test": "^8.6.14" - Is part of core storybook package now
Removed unnecessary storybook addons that is part of storybook@9.
Migrate packages tests that used jest to vitest, Removed vite configurations and created vitest.
Updated pnpm to latest version stable.
[NOW]
[BEFORE]
Summary by CodeRabbit