Add Vue integration package#181
Conversation
|
@tankylz is attempting to deploy a commit to the cstrnt's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR adds a new ChangesVue Integration Package
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/vue/tests/createAbby.test.ts (1)
116-124: ⚡ Quick winUse
AbbyEventType.ACTinstead of the literal1.Coupling the assertion to the numeric value of the enum makes the test fragile to reordering of
AbbyEventTypemembers in@tryabby/coreand obscures intent at the call site.♻️ Proposed change
-import { HttpService } from "@tryabby/core"; +import { AbbyEventType, HttpService } from "@tryabby/core"; @@ - expect(sendData).toHaveBeenCalledWith( - expect.objectContaining({ - type: 1, - data: expect.objectContaining({ - projectId: "project", - testName: "buttonCopy", - }), - }) - ); + expect(sendData).toHaveBeenCalledWith( + expect.objectContaining({ + type: AbbyEventType.ACT, + data: expect.objectContaining({ + projectId: "project", + testName: "buttonCopy", + }), + }) + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/tests/createAbby.test.ts` around lines 116 - 124, The assertion uses the numeric literal 1 for the event type which is fragile; import AbbyEventType from "@tryabby/core" and replace the literal with AbbyEventType.ACT in the expect in createAbby.test.ts (the expect(... sendData ... objectContaining { type: ... } block) so the test asserts using the enum value instead of the numeric literal, improving clarity and stability.packages/vue/src/StorageService.ts (1)
12-67: ⚡ Quick winConsolidate three near-identical cookie storage classes.
The three classes are virtually identical apart from the storage-key function. A single generic class parameterized by the key builder would eliminate duplication and reduce future maintenance cost (e.g., changing cookie attributes/SameSite/Secure would currently need to be done in three places).
♻️ Proposed refactor
-class ABStorageService implements IStorageService { - get(projectId: string, testName: string): string | null { - return Cookie.get(getABStorageKey(projectId, testName)) ?? null; - } - - set( - projectId: string, - testName: string, - value: string, - options?: StorageServiceOptions - ): void { - Cookie.set(getABStorageKey(projectId, testName), value, { - expires: options?.expiresInDays ?? DEFAULT_COOKIE_AGE, - }); - } - - remove(projectId: string, testName: string): void { - Cookie.remove(getABStorageKey(projectId, testName)); - } -} - -class FFStorageService implements IStorageService { - get(projectId: string, flagName: string): string | null { - return Cookie.get(getFFStorageKey(projectId, flagName)) ?? null; - } - - set(projectId: string, flagName: string, value: string): void { - Cookie.set(getFFStorageKey(projectId, flagName), value, { - expires: DEFAULT_COOKIE_AGE, - }); - } - - remove(projectId: string, flagName: string): void { - Cookie.remove(getFFStorageKey(projectId, flagName)); - } -} - -class RCStorageService implements IStorageService { - get(projectId: string, key: string): string | null { - return Cookie.get(getRCStorageKey(projectId, key)) ?? null; - } - - set(projectId: string, key: string, value: string): void { - Cookie.set(getRCStorageKey(projectId, key), value, { - expires: DEFAULT_COOKIE_AGE, - }); - } - - remove(projectId: string, key: string): void { - Cookie.remove(getRCStorageKey(projectId, key)); - } -} - -export const TestStorageService = new ABStorageService(); -export const FlagStorageService = new FFStorageService(); -export const RemoteConfigStorageService = new RCStorageService(); +class CookieStorageService implements IStorageService { + constructor( + private readonly buildKey: (projectId: string, name: string) => string + ) {} + + get(projectId: string, name: string): string | null { + return Cookie.get(this.buildKey(projectId, name)) ?? null; + } + + set( + projectId: string, + name: string, + value: string, + options?: StorageServiceOptions + ): void { + Cookie.set(this.buildKey(projectId, name), value, { + expires: options?.expiresInDays ?? DEFAULT_COOKIE_AGE, + }); + } + + remove(projectId: string, name: string): void { + Cookie.remove(this.buildKey(projectId, name)); + } +} + +export const TestStorageService = new CookieStorageService(getABStorageKey); +export const FlagStorageService = new CookieStorageService(getFFStorageKey); +export const RemoteConfigStorageService = new CookieStorageService(getRCStorageKey);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vue/src/StorageService.ts` around lines 12 - 67, The three nearly identical classes ABStorageService, FFStorageService, and RCStorageService should be replaced by a single generic implementation (e.g., GenericCookieStorage) that implements IStorageService and accepts a keyBuilder function (the existing getABStorageKey, getFFStorageKey, getRCStorageKey) in its constructor; implement get/set/remove to call keyBuilder(projectId, name) and use the same cookie options (honoring options?.expiresInDays in set), then export TestStorageService, FlagStorageService, and RemoteConfigStorageService as instances of GenericCookieStorage constructed with the corresponding key builders so future cookie attribute changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vue/src/index.ts`:
- Around line 149-152: The subscription to abby.subscribe is being created
during setup (which runs in SSR) and cleaned up with onBeforeUnmount (which
doesn't run on the server), causing per-request leaks; move the
subscribe/unsubscribe pair into onMounted so the subscription is only created on
the client: inside setup keep the data ref, but call abby.subscribe(...) and
register the returned unsubscribe inside onMounted; remove/avoid creating the
subscription at top-level of setup and ensure the onBeforeUnmount cleanup
remains paired with the same unsubscribe reference created in onMounted.
---
Nitpick comments:
In `@packages/vue/src/StorageService.ts`:
- Around line 12-67: The three nearly identical classes ABStorageService,
FFStorageService, and RCStorageService should be replaced by a single generic
implementation (e.g., GenericCookieStorage) that implements IStorageService and
accepts a keyBuilder function (the existing getABStorageKey, getFFStorageKey,
getRCStorageKey) in its constructor; implement get/set/remove to call
keyBuilder(projectId, name) and use the same cookie options (honoring
options?.expiresInDays in set), then export TestStorageService,
FlagStorageService, and RemoteConfigStorageService as instances of
GenericCookieStorage constructed with the corresponding key builders so future
cookie attribute changes are made in one place.
In `@packages/vue/tests/createAbby.test.ts`:
- Around line 116-124: The assertion uses the numeric literal 1 for the event
type which is fragile; import AbbyEventType from "@tryabby/core" and replace the
literal with AbbyEventType.ACT in the expect in createAbby.test.ts (the
expect(... sendData ... objectContaining { type: ... } block) so the test
asserts using the enum value instead of the numeric literal, improving clarity
and stability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 000a83b6-8ea7-4be2-8d84-e83a054a9d18
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.changeset/clean-vue-integration.mdpackages/vue/README.mdpackages/vue/package.jsonpackages/vue/src/StorageService.tspackages/vue/src/index.tspackages/vue/tests/createAbby.test.tspackages/vue/tsconfig.jsonpackages/vue/vitest.config.ts
|
Addressed the CodeRabbit SSR lifecycle feedback in 75b5a26: the provider now creates the Abby subscription only after client mount and keeps the cleanup registered through the component unmount hook. Also replaced the magic ACT event number in the focused test with the core enum.\n\nVerification:\n- pnpm --filter @tryabby/vue exec vitest run\n- pnpm --filter @tryabby/vue build\n- pnpm exec biome check packages/vue/src packages/vue/tests packages/vue/vitest.config.ts packages/vue/README.md |
|
Polished the remaining storage-service nit in aee61aa by replacing the three duplicate cookie storage classes with a single keyed implementation while preserving the exported services and expiration behavior.\n\nVerification after the update:\n- \pnpm --filter @tryabby/vue exec vitest run\n- \pnpm --filter @tryabby/vue build\n- \pnpm exec biome check packages/vue/src packages/vue/tests packages/vue/vitest.config.ts packages/vue/README.md\n\nAll passed locally. The only local note was the existing Node engine warning because this machine is on Node v20.19.4 while the repo asks for >=22.8.0. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/vue/src/StorageService.ts`:
- Around line 17-19: The StorageService methods lack an explicit SSR guard;
update get (StorageService.get), set (StorageService.set) and remove
(StorageService.remove) to check if typeof window === "undefined" before using
Cookie: have get return null immediately when window is undefined, and have
set/remove be no-ops (return void) when window is undefined, while still using
this.buildKey(projectId, name) for keys when running in the browser; keep
existing Cookie usage unchanged for the browser path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 46cac539-6518-447c-9235-cca1e1a7688c
📒 Files selected for processing (3)
packages/vue/src/StorageService.tspackages/vue/src/index.tspackages/vue/tests/createAbby.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/vue/tests/createAbby.test.ts
- packages/vue/src/index.ts
|
Addressed the follow-up SSR storage guard feedback in c0d06cd: the shared Vue cookie storage now returns null for reads and no-ops writes/removes when running without a browser window, while preserving the browser cookie path unchanged.\n\nVerification after the update:\n- \pnpm --filter @tryabby/vue exec vitest run\n- \pnpm --filter @tryabby/vue build\n- \pnpm exec biome check packages/vue/src packages/vue/tests packages/vue/vitest.config.ts packages/vue/README.md\n\nAll passed locally. Same local environment note as before: Node v20.19.4 reports the repo's >=22.8.0 engine warning. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Added a short package walkthrough video for the bounty/review evidence: https://github.com/Iineman2/abby/releases/download/abby-pr181-vue-package-demo/abby-pr181-vue-package-demo.mp4 Current maintenance status from my side: the CodeRabbit feedback has been addressed, local package tests/build/biome passed as noted above, and the remaining failing Vercel status appears to be the usual fork deployment authorization gate requiring a team member to approve or ignore the preview. |
2 participants
Summary
@tryabby/vuepackage underpackages/vueDemo video
Verification
pnpm install --filter @tryabby/vue... --prefer-offlinepnpm --filter @tryabby/core buildpnpm --filter @tryabby/vue exec vitest runpnpm --filter @tryabby/vue buildpnpm exec biome check packages/vue/src packages/vue/tests packages/vue/vitest.config.ts packages/vue/README.mdgit diff --check/claim #68
Summary by CodeRabbit
New Features
Documentation
Tests