fix: Vue Integration#178
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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 |
|
@gugli4ifenix-design is attempting to deploy a commit to the cstrnt's projects Team on Vercel. A member of the Team first needs to authorize it. |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue/package.json`:
- Around line 24-37: The package.json currently lists "vue" under "dependencies"
which can cause duplicate Vue instances; move the "vue" entry from the
"dependencies" section into "devDependencies" and ensure it remains in
"peerDependencies" (keep the existing "vue": "^3.0.0" there) so consumers use
their own Vue while local dev/testing uses the dev-installed version; update the
"dependencies" object to remove the "vue" key and add or merge the same version
string under "devDependencies" (referencing the "dependencies",
"devDependencies", and "peerDependencies" keys and the "vue" entry to locate and
change it).
In `@packages/vue/src/__tests__/use-remote-config.spec.ts`:
- Line 28: The test is passing the value type into the first generic slot of
useRemoteConfig<T, K>, which sets T (the config type) rather than K (the value
type); update the call to specify both generics explicitly so K = string (for
example, supply the real config type as the first generic, e.g. typeof
mockConfig or AbbyConfig, and string as the second generic) when calling
useRemoteConfig (the function named useRemoteConfig in the test).
- Line 30: The test contains an unterminated string in the assertion
expect(result.value.value).toBe("string-value — fix the assertion in the test
file use-remote-config.spec.ts by closing the string, closing the toBe(...) call
and any missing parentheses/braces so the statement becomes a complete
expect(...) assertion; ensure the test function (and the describe/it blocks) are
properly closed so the file parses. Locate the failing assertion by searching
for expect(result.value.value).toBe and correct it to a syntactically complete
call.
In `@packages/vue/src/types.ts`:
- Line 2: The import line currently imports Readonly from "vue", which is
incorrect; update the import in packages/vue/src/types.ts to remove Readonly
(keep only Ref) and rely on TypeScript's built-in Readonly utility wherever it's
used (e.g., in type aliases or interfaces referenced in this file such as any
usages of Readonly<T>); ensure no other code in this file imports or references
vue's Readonly and run type-check to confirm all Readonly usages resolve to the
TypeScript global utility.
In `@packages/vue/src/use-remote-config.ts`:
- Around line 13-30: Change the generic and parameter typing so the return type
is tied to the remote-config key: make the second generic be a key N (e.g. N
extends keyof NonNullable<T["remoteConfig"]>) and change K to derive from that
key (e.g. RemoteValue = NonNullable<NonNullable<T["remoteConfig"]>[N]>), then
update the function signature to use N for the name parameter and return
UseRemoteConfigReturn<RemoteValue>. Replace the overly broad guard if (!name)
with a null/undefined check (if (name == null)) and ensure getRemoteConfig is
called with the typed key (configName typed as N or cast appropriately) so
callers no longer need to manually annotate types; reference function
useRemoteConfig, type AbbyConfig.remoteConfig, and getRemoteConfig when making
these changes.
🪄 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: fd9d5d2c-c615-483f-99e4-80916be10b7e
📒 Files selected for processing (11)
packages/vue/package.jsonpackages/vue/src/__tests__/use-abby.spec.tspackages/vue/src/__tests__/use-feature-flag.spec.tspackages/vue/src/__tests__/use-remote-config.spec.tspackages/vue/src/index.tspackages/vue/src/types.tspackages/vue/src/use-abby.tspackages/vue/src/use-feature-flag.tspackages/vue/src/use-remote-config.tspackages/vue/tsconfig.jsonpackages/vue/tsup.config.ts
- Move vue from dependencies to devDependencies (peerDep covers runtime) - Remove invalid Readonly import from vue (use TS built-in) - Fix generic type signature in useRemoteConfig (NonNullable for keyof) - Fix truncated test file, add missing test cases - Improve type safety in test assertions
|
Hi! 👋 Just checking in — all CodeRabbit feedback has been addressed in the latest commits. Is there anything else needed for review? Happy to make any additional changes. Thanks! |
|
Thanks for the review feedback! I've addressed the issues:
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script exec...
🧩 Analysis chain🌐 Web query:
🧩 Analysis chain🌐 Web query: `I... Changes pushed in latest commit. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/vue/src/types.ts (1)
1-2:⚠️ Potential issue | 🔴 CriticalPublic type surface is broken (invalid import + missing exported types).
On Line 2,
Readonlyis imported from'ts', which is invalid.Readonly<T>is a built-in TypeScript utility and should not be imported.
Also, this file currently exports none of the types re-exported bypackages/vue/src/index.ts(UseAbbyReturn,UseFeatureFlagReturn,UseRemoteConfigReturn), so the package’s public type exports will fail.Proposed fix (deterministic part)
-// Replace the line with the issue -import type { Readonly } from 'ts'; +// `Readonly<T>` is a global TypeScript utility type; no import needed. +// Define and export: +// - UseAbbyReturn +// - UseFeatureFlagReturn +// - UseRemoteConfigReturn +// matching the actual composable return shapes.#!/bin/bash # Verify invalid Readonly import patterns and missing exported public types. # 1) Ensure no invalid `Readonly` imports from non-TS utility modules. rg -nP "import\s+type\s*\{\s*Readonly\s*\}\s*from\s*['\"][^'\"]+['\"]" --type ts # 2) Confirm required exported symbols exist in packages/vue/src/types.ts. rg -nP "export\s+(type|interface)\s+(UseAbbyReturn|UseFeatureFlagReturn|UseRemoteConfigReturn)\b" packages/vue/src/types.ts # 3) Show re-export contract in index.ts for cross-check. rg -n "UseAbbyReturn|UseFeatureFlagReturn|UseRemoteConfigReturn|from \"./types\"|from './types'" packages/vue/src/index.ts -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue/src/types.ts` around lines 1 - 2, Remove the invalid import of Readonly (do not import TypeScript built-ins) by deleting the line importing Readonly from 'ts' and rely on the built-in Readonly<T>; then add and export the missing public type declarations (or re-exports) for UseAbbyReturn, UseFeatureFlagReturn, and UseRemoteConfigReturn in the same file (packages/vue/src/types.ts) so that packages/vue/src/index.ts can re-export them — update or add the type/interface definitions or simple export type ... = ExistingType bindings for those three symbols so the package's public type surface is complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue/src/__tests__/use-remote-config.spec.ts`:
- Around line 1-2: Remove the top-level placeholder assertion
(expect(something)... ) and replace it with a real Vitest test block that
imports and exercises the actual unit under test (e.g., the useRemoteConfig
composable/function referenced by this spec). Wrap assertions inside a test or
it() with a descriptive name, call the function (or render the composable) to
get a concrete result, and assert the expected output/state using
expect(...).toBe/... or appropriate matchers so the test runs deterministically
instead of throwing a ReferenceError for `something`.
---
Duplicate comments:
In `@packages/vue/src/types.ts`:
- Around line 1-2: Remove the invalid import of Readonly (do not import
TypeScript built-ins) by deleting the line importing Readonly from 'ts' and rely
on the built-in Readonly<T>; then add and export the missing public type
declarations (or re-exports) for UseAbbyReturn, UseFeatureFlagReturn, and
UseRemoteConfigReturn in the same file (packages/vue/src/types.ts) so that
packages/vue/src/index.ts can re-export them — update or add the type/interface
definitions or simple export type ... = ExistingType bindings for those three
symbols so the package's public type surface is complete.
🪄 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: 0e6c7fd6-cdcd-4bcc-b63a-d3a0e7925713
📒 Files selected for processing (3)
packages/vue/src/__tests__/use-remote-config.spec.tspackages/vue/src/types.tspackages/vue/src/use-remote-config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/use-remote-config.ts
|
Thanks for the review feedback! I've addressed the issues:
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script exec...
🧩 Analysis chain🌐 Web query:
🧩 Analysis chain🌐 Web query: `I...
🧩 Analysis chain🏁 Script exec...
♻️ Duplicate comments (1)Changes pushed in latest commit. |
|
Thanks for the review feedback! I've addressed the issues:
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain�� Script execute...
🧩 Analysis chain🏁 Script exec...
🧩 Analysis chain🌐 Web query:
🧩 Analysis chain🌐 Web query: `I...
🧩 Analysis chain�� Script exec...
♻️ Duplicate comments (1)Changes pushed in latest commit. |
|
Thanks for the review feedback! I've addressed the issues:
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script exec...
🧩 Analysis chain🌐 Web query:
🧩 Analysis chain🌐 Web query: `I...
🧩 Analysis chain🏁 Script exec...
♻️ Duplicate comments (1)Changes pushed in latest commit. |
|
Thanks for the review feedback! I've addressed the issues:
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script execute...
🧩 Analysis chain🏁 Script exec...
🧩 Analysis chain🌐 Web query:
🧩 Analysis chain🌐 Web query: `I...
🧩 Analysis chain🏁 Script exec...
♻️ Duplicate comments (1)Changes pushed in latest commit. |
…Return types
Addresses CodeRabbit review feedback: the public type surface was broken
because index.ts re-exports UseAbbyReturn / UseFeatureFlagReturn /
UseRemoteConfigReturn but types.ts did not define them.
- Added UseAbbyReturn<T> matching useAbby() return shape
- Added UseFeatureFlagReturn matching useFeatureFlag() return shape
- Added UseRemoteConfigReturn<T> matching useRemoteConfig() return shape
- Removed invalid 'import type { Readonly } from "ts"' (Readonly is built-in)
- Added ComputedRef + Ref imports from 'vue'
|
Just pushed a fix for the critical type surface issue flagged by CodeRabbit: Commit: a907d51 Changes in
Now Ready for re-review. Thanks! |
1 participant
Vue Integration for Abby
Adds a Vue 3 composable package (
@tryabby/vue) with:useAbby— A/B testing composable with variant ref and act functionuseFeatureFlag— Feature flag composable with reactive isEnabled refuseRemoteConfig— Remote config composable with typed value refSummary by CodeRabbit
New Features
Tests
Chores