✨ [FFL-2596] Feature Flags tab — overrides (stacked PR 3 of 3) - #4912
✨ [FFL-2596] Feature Flags tab — overrides (stacked PR 3 of 3)#4912kellyw1806 wants to merge 1 commit into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
kellyw1806
changed the title
f6766af to
5375214
Compare
17cca80 to
5b4061e
Compare
Bundles Sizes Evolution
|
|
I have read the CLA Document and I hereby sign the CLA |
5b4061e to
31f7bb0
Compare
kellyw1806
changed the title
5375214 to
bc5e1d9
Compare
31f7bb0 to
c9960d2
Compare
kellyw1806
changed the title
| // Contract shared with @datadog/openfeature-browser's DatadogDevtools wrapper. | ||
| // Keep these in sync with that package: the wrapper reads OVERRIDES_KEY once on | ||
| // initialize() and writes DEVTOOLS_MARKER_KEY when it is composed into the provider stack. | ||
| export const OVERRIDES_KEY = 'dd.dd_flag.overrides' |
There was a problem hiding this comment.
These keys are a contract with @datadog/openfeature-browser's DatadogDevtools wrapper so renaming them requires a coordinated change in that package, not a free rename.
| type: FlagOverrideType | ||
| // Any JSON value — objects/arrays are `object`; `null` is allowed (a flag/variant value can be | ||
| // null). The manual-entry form still rejects null via validateOverrideValue. | ||
| value: boolean | string | number | object | null |
There was a problem hiding this comment.
Widened the override value to allow null (a variant value can legitimately be null). The manual-entry form still rejects null via validateOverrideValue. Is this okay or should variant-click also block null?
|
|
||
| useEffect(() => { | ||
| refresh() | ||
| const id = setInterval(refresh, REFRESH_INTERVAL) |
There was a problem hiding this comment.
This polls the inspected page every 2s while the tab is mounted. Fine for a devtools panel IMO, but flagging in case prefer gating it on tab visibility.
bc5e1d9 to
2e87cbc
Compare
c9960d2 to
fc8bff4
Compare
2e87cbc to
7b347cd
Compare
fc8bff4 to
5d030ab
Compare
kellyw1806
changed the title
17d898b to
919073f
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e13445b763
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case 'INTEGER': | ||
| case 'NUMERIC': { | ||
| const text = String(raw).trim() | ||
| if (text === '' || Number.isNaN(Number(text))) { |
There was a problem hiding this comment.
issue: For a manual NUMERIC override, inputs such as Infinity or 1e9999 pass this Number.isNaN check and become a non-finite number. writeOverrides subsequently serializes that value with JSON.stringify, which converts it to null; after refreshing, the provider receives an invalid null override instead of the number the UI accepted. Validate with Number.isFinite before applying the override.
Useful? React with 👍 / 👎.
| await writeOverrides(mutate(overrides)) | ||
| refresh() | ||
| }) | ||
| .catch((error) => logger.error('Error while updating flag overrides:', error)) |
There was a problem hiding this comment.
issue: When localStorage.setItem fails—for example because storage is unavailable or over quota—this catch logs the error and converts the mutation into a fulfilled promise. FlagsTab.runMutation then considers the write settled while leaving pendingReload true, so the UI enables “Save Overrides and Refresh Page” even though nothing was saved; refreshing silently applies the old overrides. Preserve a failure signal and keep the refresh action from representing the failed mutation as saved.
Useful? React with 👍 / 👎.
| </Badge> | ||
| )) | ||
| flag.variants.map((variant) => { | ||
| const isActive = overridden && valuesEqual(override.value, variant.value) |
There was a problem hiding this comment.
issue: If the shared local-storage key contains syntactically valid JSON but an invalid entry such as {"my-flag": null}, readFlagState accepts the map and getOverride returns that null entry. This row then treats it as overridden and dereferences override.value, crashing the entire extension panel through its error boundary. Validate or discard each stored entry rather than trusting only the top-level object shape.
Useful? React with 👍 / 👎.
| const { overrides } = await readFlagState() | ||
| await writeOverrides(mutate(overrides)) |
There was a problem hiding this comment.
issue: When the inspected page navigates between these two asynchronous evalInWindow round trips, the read can capture overrides from the old page while the write runs against the new page, copying the old origin's override map into the new origin along with the requested change. The same split read/write also overwrites changes made by another page-side writer between the calls. Perform the read-modify-write in one inspected-window evaluation so it is bound to a single page context.
Useful? React with 👍 / 👎.
| const totalPages = Math.max(1, Math.ceil(sorted.length / CATALOG_PAGE_SIZE)) | ||
| const currentPage = Math.min(page, totalPages) | ||
| const paginated = filtered.slice((currentPage - 1) * CATALOG_PAGE_SIZE, currentPage * CATALOG_PAGE_SIZE) | ||
| const paginated = sorted.slice((currentPage - 1) * CATALOG_PAGE_SIZE, currentPage * CATALOG_PAGE_SIZE) |
There was a problem hiding this comment.
issue: In catalogs with more than one page, applying an override while viewing page 2 or later immediately moves that flag into the overridden group at the start of the sorted array, but the current page is retained. The clicked row therefore disappears before the user can see its active state and is replaced by a different flag shifted from the previous page. Reset pagination when override membership changes or avoid moving the edited row away from the current page.
Useful? React with 👍 / 👎.
|
|
||
| <FlagCatalogBody | ||
| catalog={catalog} | ||
| flags={view.paginated} |
There was a problem hiding this comment.
issue: When ManualOverrideForm adds a key that is absent from the fetched catalog—the primary custom-override case—the new entry can never appear in this catalog-only list. The UI shows only an increased “Clear all” count, so users cannot inspect the persisted type/value or remove that custom entry without deleting every other override. Render non-catalog entries with their own revert control.
Useful? React with 👍 / 👎.
5683248 to
4af8227
Compare
e13445b to
d3669d4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3669d4516
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| useEffect(() => { | ||
| refresh() | ||
| const id = setInterval(refresh, REFRESH_INTERVAL) | ||
| return () => clearInterval(id) |
There was a problem hiding this comment.
issue: When the inspected tab navigates to a different origin while this panel remains open, the previous origin's overrides and marker remain rendered until the two-second poll completes. During that window, clicking a displayed revert or “Clear all” performs a fresh read against the new origin and can delete overrides there that were never shown to the user. Listen for top-frame navigation and clear or refresh the state immediately before mutation controls remain usable.
Useful? React with 👍 / 👎.
| const view = useFlagCatalogView(catalog.flags) | ||
| const view = useFlagCatalogView(catalog.flags, overrides) | ||
|
|
||
| const [pendingReload, setPendingReload] = useState(false) |
There was a problem hiding this comment.
issue: If the panel is destroyed and reopened after an override mutation but before refreshing the inspected page, this transient state resets to false even though the wrapper still has its pre-mutation values loaded. The persisted overrides are displayed again, but “Save Overrides and Refresh Page” is disabled; after clearing overrides this can leave the page running an old override while the panel shows none. Do not gate the required refresh solely on component-local state, or persist the dirty state until the inspected page reloads.
Useful? React with 👍 / 👎.
| size="compact-xs" | ||
| variant={isActive ? 'filled' : 'default'} | ||
| color={isActive ? 'violet' : 'gray'} | ||
| onClick={() => onSelectVariant(flag.key, { type: flag.type, value: variant.value })} |
There was a problem hiding this comment.
issue: When the catalog API supplies an unparseable variant, parseVariantValue deliberately preserves its raw string (the existing catalog spec covers malformed JSON and INTEGER variants), but this click handler now persists that fallback under the declared non-string type. Such an override violates validateOverrideValue and can make the provider throw when resolving the flag after refresh. Validate catalog values before applying them and disable or report malformed variants instead of writing them.
Useful? React with 👍 / 👎.
4af8227 to
7376fbf
Compare
d3669d4 to
bad0da0
Compare
7376fbf to
fb4faa6
Compare
bad0da0 to
683bd8b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 683bd8bf66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setError(validationError) | ||
| return | ||
| } | ||
| onApply(flagKey.trim(), { type, value }) |
There was a problem hiding this comment.
issue: When this form is used with a key that already exists in catalog.flags, it still allows an unrelated type to be selected and persists that type without comparing it with the known CatalogFlag.type. For example, a STRING override for a catalogued BOOLEAN flag passes validateOverrideValue, but the DatadogDevtools wrapper resolves that key through the boolean path and rejects the mismatched override instead of applying it after refresh. Use the catalog type, or reject a mismatch, whenever the entered key is known.
Useful? React with 👍 / 👎.
| overrides = parsed | ||
| } | ||
| } catch (e) {} | ||
| const devtoolsEnabled = localStorage.getItem(${JSON.stringify(DEVTOOLS_MARKER_KEY)}) === 'enabled' |
There was a problem hiding this comment.
issue: When an origin previously ran a build containing DatadogDevtools but the current build no longer composes it, this localStorage marker remains set indefinitely because nothing here clears or scopes it to a document load. The panel therefore suppresses the “not detected” warning and presents overrides as supported even though refreshing cannot apply them. Use a marker tied to the current page lifecycle, or clear and re-establish it during reload, before reporting the wrapper as detected.
Useful? React with 👍 / 👎.
fb4faa6 to
230ae07
Compare
683bd8b to
1f5d159
Compare
230ae07 to
673a2b1
Compare
1f5d159 to
37d6b9b
Compare
673a2b1 to
2f033a5
Compare
37d6b9b to
b1988f2
Compare
2f033a5 to
98058ac
Compare
b1988f2 to
c9cefb8
Compare
98058ac to
e55866f
Compare
c9cefb8 to
3ef64a4
Compare
e55866f to
322814a
Compare
3ef64a4 to
e97cfa9
Compare
322814a to
87a1c1a
Compare
e97cfa9 to
6692837
Compare
Builds on FFL-2597: adds the override engine (writes to the inspected page's localStorage via the DatadogDevtools contract), per-variant override buttons + revert on each catalog row, a manual override-by-key form, and clear-all / save-and-reload controls.
87a1c1a to
875de0b
Compare
6692837 to
cf6c4db
Compare
Motivation
Stacked on #4916 (catalog fetch). Allows users to create overrides for a flag's value (allows for switching between variables) locally and have it take effect on the inspected page.
Stack (review bottom-up)
mainffl-2597ffl-2858Changes
useFlagOverrides) writing to the inspected page'slocalStorageper theDatadogDevtoolswrapper contract; mutations serialized to avoid lost updates.Demo
This is a recording of what the three stacked PRs look like working together.
Screen.Recording.2026-07-24.at.5.14.31.PM.mov
Checklist
useFlagOverrides.spec.ts).