chore: replace usages with @vibe/shared#3340
Conversation
Replace all internal @vibe/core imports of utils and hooks that were
already copied to @vibe/shared with direct imports from @vibe/shared.
Changes:
- Add `testid-helper` to @vibe/shared public exports
- Replace local bridge imports in ~120 files across components and hooks
with direct imports from @vibe/shared (useMergeRef, useIsomorphicLayoutEffect,
useIsMounted, getStyle, NOOP, chainFunctions, chainRefFunctions,
convertToArray, isFirefox, isClient, isEnterEvent, isEscapeEvent, etc.)
- Delete 13 now-redundant bridge files from @vibe/core:
- utils: ssr-utils, dom-utils, dom-event-utils, warn-deprecated,
user-agent-utils, media-query-utils, function-utils
- helpers: testid-helper, typesciptCssModulesHelper, screenReaderAccessHelper
- hooks: useMergeRef, ssr/useIsomorphicLayoutEffect, ssr/useIsMounted
- Update Checkbox test vi.mock to use importOriginal pattern so partial
mocking of @vibe/shared doesn't break other consumers
Review Summary by QodoReplace internal @vibe/core imports with direct @vibe/shared imports
WalkthroughsDescription• Replace ~120 local bridge imports with direct @vibe/shared imports • Delete 13 redundant bridge files that were pure re-exports • Add testid-helper to @vibe/shared public exports • Fix Checkbox test mocking to use importOriginal pattern Diagramflowchart LR
A["@vibe/core components<br/>and hooks"] -->|"Replace local<br/>bridge imports"| B["@vibe/shared"]
C["13 redundant<br/>bridge files"] -->|"Delete"| D["Removed"]
E["testid-helper"] -->|"Export"| B
F["Checkbox test"] -->|"Fix vi.mock<br/>with importOriginal"| G["Partial mocking"]
File Changes1. packages/core/src/components/List/utils/ListContext.ts
|
Code Review by Qodo
1. Hoisted mock in hooks
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| beforeAll(() => { | ||
| vi.mock("../../../utils/user-agent-utils", () => { | ||
| vi.mock("@vibe/shared", async importOriginal => { | ||
| const actual = await importOriginal(); | ||
| return { | ||
| ...(actual as object), | ||
| isFirefox: vi.fn().mockImplementation(() => true) | ||
| }; | ||
| }); | ||
| }); | ||
|
|
||
| afterAll(() => { | ||
| vi.mock("../../../utils/user-agent-utils", () => { | ||
| vi.mock("@vibe/shared", async importOriginal => { | ||
| const actual = await importOriginal(); | ||
| return { | ||
| ...(actual as object), | ||
| isFirefox: vi.fn() | ||
| }; | ||
| }); |
There was a problem hiding this comment.
1. Hoisted mock in hooks 🐞 Bug ✓ Correctness
Checkbox.test.tsx calls vi.mock("@vibe/shared") inside beforeAll/afterAll, but vi.mock is
hoisted and only affects module evaluation time, so these runtime calls won’t reliably change
isFirefox for the already-imported Checkbox. This can cause the “firefox checkbox tests” to run
against the default mocked isFirefox instead of true, making the suite misleading/flaky.
Agent Prompt
### Issue description
`vi.mock()` is hoisted and intended to be declared at module scope. The Firefox-specific tests currently call `vi.mock("@vibe/shared")` inside `beforeAll/afterAll`, which won’t reliably change `isFirefox` for `Checkbox` (already imported).
### Issue Context
You already have a top-level partial mock of `@vibe/shared` that defines `isFirefox: vi.fn()`. Reuse that single mock and adjust its return value per test block.
### How to fix
- Remove the `vi.mock("@vibe/shared", ...)` calls from `beforeAll`/`afterAll`.
- Import the mocked export and change its implementation instead.
- Recommended pattern:
```ts
import * as shared from "@vibe/shared";
vi.mock("@vibe/shared", async importOriginal => {
const actual = await importOriginal();
return { ...(actual as object), isFirefox: vi.fn() };
});
// in firefox describe
beforeAll(() => {
vi.mocked(shared.isFirefox).mockReturnValue(true);
});
afterAll(() => {
vi.mocked(shared.isFirefox).mockReset();
vi.mocked(shared.isFirefox).mockReturnValue(false); // optional explicit default
});
```
- Alternatively set `mockReturnValue(true)` in `beforeEach` for tighter isolation.
### Fix Focus Areas
- packages/core/src/components/Checkbox/__tests__/Checkbox.test.tsx[1-12]
- packages/core/src/components/Checkbox/__tests__/Checkbox.test.tsx[270-288]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
📦 Bundle Size Analysis ✅ No bundle size changes detected. Unchanged Components
📊 Summary:
|
2 participants
Summary
Eliminates code duplication between
@vibe/coreand@vibe/sharedby replacing all internal imports that were using local bridge files with direct imports from@vibe/shared.testid-helperto@vibe/sharedpublic exports (existed but wasn't exported)@vibe/corecomponents and hooks with direct@vibe/sharedimports (useMergeRef,useIsomorphicLayoutEffect,getStyle,NOOP,chainFunctions,isFirefox,isClient, keyboard event helpers, etc.)@vibe/corethat were pure re-exports of@vibe/shared:utils/:ssr-utils,dom-utils,dom-event-utils,warn-deprecated,user-agent-utils,media-query-utils,function-utilshelpers/:testid-helper,typesciptCssModulesHelper,screenReaderAccessHelperhooks/:useMergeRef,ssr/useIsomorphicLayoutEffect,ssr/useIsMountedCheckboxtestvi.mockto useimportOriginalpattern for partial mocking of@vibe/sharedNo public API changes —
useEventListeneranduseKeyEventremain exported from@vibe/corevia their existing bridge files (which now delegate to@vibe/shared).Test plan
yarn workspace @vibe/core test --run— 141/141 passingyarn workspace @vibe/core lint— 0 errorsyarn workspace @vibe/core build— clean buildTicket: https://monday.monday.com/boards/3532714909/pulses/11161238492