refactor(featureFlags): centralize ?ff= reading with the other query-state readers - #15044
Conversation
🎭 Playwright: ✅ 1804 passed, 0 failed · 1 flaky📊 Browser Reports
🎨 Storybook: ✅ Built — View Storybook📦 Bundle: 8.69 MB gzip 🔴 +345 BDetailsSummary
Category Glance App Entry Points — 3.67 kB (baseline 3.67 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.36 MB (baseline 1.36 MB) • 🔴 +1 BGraph editor runtime, canvas, workflow orchestration
Status: 2 added / 2 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • 🔴 +2 BTop-level views, pages, and routed surfaces
Status: 16 added / 16 removed / 1 unchanged Panels & Settings — 565 kB (baseline 565 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 27 kB (baseline 27 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 8 added / 8 removed / 2 unchanged Editors & Dialogs — 125 kB (baseline 125 kB) • 🔴 +2 BModals, dialogs, drawers, and in-app editors
Status: 8 added / 8 removed UI Components — 67.1 kB (baseline 67.1 kB) • ⚪ 0 BReusable component library chunks
Status: 14 added / 14 removed Data & Services — 3.5 MB (baseline 3.5 MB) • 🔴 +1 BStores, services, APIs, and repositories
Status: 17 added / 17 removed Utilities & Hooks — 550 kB (baseline 550 kB) • 🔴 +1 BHelpers, composables, and utility bundles
Status: 28 added / 28 removed / 10 unchanged Vendor & Third-Party — 16.3 MB (baseline 16.3 MB) • 🔴 +22 BExternal libraries and shared vendor chunks
Status: 6 added / 6 removed / 11 unchanged Other — 14 MB (baseline 14 MB) • 🔴 +10 BBundles that do not match a named category
Status: 113 added / 113 removed / 172 unchanged ⚡ Performance
|
📝 WalkthroughWalkthroughThe session feature-flag override utility now uses Vue Router’s ChangesFeature flag query capture
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #15044 +/- ##
========================================
Coverage 81.32% 81.33%
========================================
Files 1873 1873
Lines 106556 106556
Branches 33382 32841 -541
========================================
+ Hits 86661 86662 +1
+ Misses 19549 19538 -11
- Partials 346 356 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Thanks — no actionable comments, and nothing to change. Recording where the one unresolved pre-merge check landed so it isn't mistaken for an open issue. "End-To-End Regression Coverage For Fixes" — inconclusive, not failed. Its stated reason is metadata, not code: "the PR title and commit subjects are not provided." The check looks for a bug-fix signal to decide whether regression coverage is required. This PR is a pure refactor — the title and the single commit subject are both That said, "it's only a refactor" is exactly the claim that deserves evidence rather than assertion, so the three behaviours this change could plausibly have broken were mutation-tested — each mutation fails exactly the intended test and nothing else:
Restored, the file is 31/31 green. CI is green on One thing a human still needs to do: this PR needs the |
dff2347 to
359fc51
Compare
|
Thanks both. Both approvals came through with an empty review body and no inline comments (the PR reports Correction: my earlier comment above is stale, please don't read it as describing this diff. It describes the previous design — a const value = parseQuery(search)[QUERY_PARAM]
if (value === undefined) return []
return (Array.isArray(value) ? value : [value]).map((value) => value ?? '')"It's only a parser swap" is exactly the claim that deserves evidence, since
Relatedly, the Verification on CodeRabbit's one unresolved pre-merge check, "End-To-End Regression Coverage For Fixes", is inconclusive rather than failed, and its stated reason is missing metadata ("does not provide commit subjects or an explicit changed-file list"), not a code finding. One thing still needing a human: this PR wants the |
Summary
Reuse Vue Router's existing query parser for
?ff=instead of parsingwindow.location.searchindependently withURLSearchParams.Changes
URLSearchParamsreader insessionFeatureFlagOverride.tswithparseQuery.?ff=parameters, nameless clearing, typed values, persistence, and employee gating.Fixes FE-1551
Verification
pnpm test:unit src/utils/sessionFeatureFlagOverride.test.tspnpm typecheckpnpm lintpnpm knippnpm exec oxfmt --check src/utils/sessionFeatureFlagOverride.ts