View UI refs #2845
View UI refs #2845
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a namespaced URL search-params mechanism ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant UI as Filter UI / FormikFilterForm
participant Params as usePrefixedSearchParams
participant Query as useAllConfigsQuery / useGetAllConfigsChangesQuery
participant API as Server/API
User->>UI: change filters / remove badge
UI->>Params: setParams(updater with prefixed keys)
Params-->>UI: updated prefixed URLSearchParams
UI->>Query: reads prefixed params -> triggers refetch
Query->>API: fetch with composed params
API-->>Query: response
Query-->>UI: data (loading false)
UI-->>User: render updated table/results
Possibly related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
adityathebe
force-pushed
the
view-ui-refs
branch
from
3c34d0c to
5829898
Compare
adityathebe
force-pushed
the
view-ui-refs
branch
from
5829898 to
d7133c4
Compare
adityathebe
force-pushed
the
view-ui-refs
branch
from
a47e06c to
8a7bca6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/components/Configs/ConfigList/Cells/ConfigListTagsCell.tsx`:
- Around line 55-79: The code reads current tags from the hard-coded "tags" key
causing custom keys to be ignored; update the logic in the setParams callback
(where setParams, nextParams, currentTags/currentTagsArray, updatedValue, and
filterByTagParamKey are used) to read using nextParams.get(filterByTagParamKey)
instead of nextParams.get("tags"), ensuring both read and write use
filterByTagParamKey so existing values under a custom param key are preserved
and de-duplicated the same way.
In `@src/components/Configs/ConfigsListFilters/ConfigGroupByDropdown.tsx`:
- Around line 112-125: The forEach callback in groupByChange currently uses an
expression body that implicitly returns the result of nextParams.delete, causing
a Biome lint warning; change the paramsToReset.forEach((param) =>
nextParams.delete(param)) to use a block body with braces and an explicit
statement (e.g., paramsToReset.forEach((param) => { nextParams.delete(param);
})) so the function no longer has an implicit return while keeping the same
behavior in groupByChange and still updating nextParams.
In `@src/hooks/usePrefixedSearchParams.ts`:
- Around line 67-87: The bug is that when useGlobalParams is true you always
write updated entries with the prefix (creating duplicate prefixed copies of
global params); fix by when useGlobalParams is true delete any prefixed variants
up front (use prefixWithSeparator and nextParams to remove keys starting with
`${prefix}__`), then when applying updatedFiltered (from filterPrefixedParams
and updater) write keys that should be global without the prefix (set key
directly on nextParams) and ensure that if the updater removed a global key you
delete its unprefixed form from nextParams so globals are not left stale.
In `@src/pages/views/components/ChangesUISection.tsx`:
- Around line 69-87: The effect that merges presetParams into the URL is gated
by isInitialized so later changes to presetParams (from new filters) don't
reapply defaults; add a new useEffect that watches presetParams and calls
setIsInitialized(false) (or otherwise resets the initialization flag) so the
merge effect runs again and setSearchParams is reapplied with the new
presetParams; ensure the new effect depends only on presetParams and uses the
same setIsInitialized function.
In `@src/pages/views/components/ViewSection.tsx`:
- Around line 43-44: Remove the contradictory non-null assertion on name:
replace the expression using section.viewRef?.name! with a plain optional result
(e.g., const name = section.viewRef?.name) and let the existing guard (enabled:
isViewRefSection && !!name in the query) handle undefined names; ensure any
downstream uses of name accept string | undefined or are already protected by
isViewRefSection/!!name (symbols: section.viewRef, name, namespace,
isViewRefSection, enabled: isViewRefSection && !!name).
In `@src/pages/views/components/ViewWithSections.tsx`:
- Around line 69-81: The generated React keys for sections (in
viewResult.sections.map) can collide when multiple sections share the same
section.title; update the key creation logic for sectionKey so it guarantees
uniqueness (e.g., detect duplicate keys or always append the map index or a
section-specific unique id if present) and use that unique value as the key on
the ViewSection wrapper to prevent React mis-diffing; adjust the code around the
sectionKey computation inside the map iterator that returns the <div
key={sectionKey}> and ensure any existing references to section.viewRef or
section.uiRef are preserved.
In `@src/ui/DataTable/FilterByCellValue.tsx`:
- Around line 30-54: The forEach callback passed to paramsToReset currently uses
an expression body which causes an implicit return from nextParams.delete(...)
and triggers the Biome useIterableCallbackReturn lint; update the callback in
the setParams block (the paramsToReset.forEach call) to use a block body (e.g.,
paramsToReset.forEach((param) => { nextParams.delete(param); });) or replace it
with an explicit loop, ensuring you reference nextParams, paramsToReset, and
paramKey in the same setParams closure.
In `@src/ui/Dates/TimeRangePicker/useTimeRangeParams.tsx`:
- Around line 28-49: The current useEffect in useTimeRangeParams uses
Object.entries(defaults) which breaks when defaults is a URLSearchParamsInit
other than a plain object; normalize defaults by creating new
URLSearchParams(defaults) and iterate with its .forEach((value, key) => { ... })
to set missing keys on nextParams so all URLSearchParamsInit types (string,
URLSearchParams, iterable, object) are handled; update the loop that currently
references Object.entries(defaults) inside the setParams callback to use this
normalized URLSearchParams and keep the same changed logic and nextParams.set
behavior.
🧹 Nitpick comments (4)
src/pages/views/components/ViewSection.tsx (1)
111-121: Consider sanitizingsection.titlein paramPrefix.Using
section.titledirectly inparamPrefix(e.g.,changes_${section.title}) could produce URL parameter keys with spaces or special characters if titles contain them. While this may work, it could cause unexpected behavior with URL encoding.Optional: sanitize title for paramPrefix
+const sanitizeForParam = (str: string) => str.replace(/[^a-zA-Z0-9_-]/g, '_'); + {section.uiRef?.changes && ( <ChangesUISection filters={section.uiRef.changes} - paramPrefix={`changes_${section.title}`} + paramPrefix={`changes_${sanitizeForParam(section.title)}`} /> )} {section.uiRef?.configs && ( <ConfigsUISection filters={section.uiRef.configs} - paramPrefix={`configs_${section.title}`} + paramPrefix={`configs_${sanitizeForParam(section.title)}`} /> )}src/pages/views/components/ConfigsUISection.tsx (1)
78-81: Consider ifcacheTime: 0is intentional.Setting
cacheTime: 0disables React Query's caching entirely, meaning every navigation or re-mount triggers a fresh fetch. If real-time data is required, this is appropriate. Otherwise, consider allowing some caching for better performance.src/pages/views/components/ChangesUIFilters.tsx (1)
134-139: Use a unique key per filter entry.
key={value}can collide across different param keys. Compose a stable key to avoid React list warnings.🔧 Suggested fix
- <FilterBadge - filters={value} - key={value} - paramKey={key} - paramPrefix={paramPrefix} - /> + <FilterBadge + filters={value} + key={`${key}:${value}`} + paramKey={key} + paramPrefix={paramPrefix} + />src/pages/views/utils/filterTranslation.ts (1)
54-55: Returnundefinedfor empty tristate output.Keeps behavior consistent with
translateTagsand avoids propagating empty strings.🔧 Suggested fix
- return result.join(","); + return result.length > 0 ? result.join(",") : undefined;
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/pages/views/components/ViewSection.tsx`:
- Around line 80-90: The header's aria-controls in renderHeader references
sectionId but the error container is using id={`${sectionId}-error`}, breaking
the aria relationship; update the error container element (the element that
currently sets id={`${sectionId}-error`} near the error rendering logic) to use
id={sectionId} so it matches aria-controls (alternatively, change aria-controls
to `${sectionId}-error`, but prefer making the error element id={sectionId} to
match renderHeader and preserve the existing sectionId variable).
- Around line 42-55: The namespace defaulting is inconsistent: change how
namespace is derived so that the variable named namespace (used in the useQuery
queryKey and passed to getViewDataByNamespace) falls back to the same "default"
value as the sectionId logic instead of an empty string; update the declaration
that sets namespace = section.viewRef?.namespace to use "default" when
undefined, ensuring useQuery (queryKey ["view-result", namespace, name,
currentViewVariables]) and the queryFn calling getViewDataByNamespace(namespace,
name, currentViewVariables) receive a valid namespace; keep the enabled
condition (isViewRefSection && !!name) unchanged.
- Around line 66-78: The sectionId and paramPrefix are built from raw
section.title and can contain spaces/punctuation; use the existing toSafeId
utility (or implement a local slugifier) to sanitize values before constructing
sectionId and paramPrefix so IDs and URL param prefixes are valid and unique;
specifically, wrap section.title (used in the ternaries that produce
`section-changes-${section.title}`, `section-configs-${section.title}`, and the
default `section-${section.title}`) with toSafeId when computing sectionId, and
similarly sanitize the strings used to build paramPrefix (the variables
referenced as paramPrefix around the props/controls) to prevent invalid DOM
id/aria-controls and collisions. Ensure you import/extract toSafeId from the
TimeseriesPanel module or add a small local implementation and update all
occurrences consistently.
In `@src/ui/Dates/TimeRangePicker/useTimeRangeParams.tsx`:
- Line 83: The forEach callback in useTimeRangeParams
(paramsToReset.forEach((param) => nextParams.delete(param))) uses an implicit
return which triggers the useIterableCallbackReturn lint error; change the arrow
function to a block body so it does not implicitly return (i.e., replace the
concise arrow callback with a block callback that calls nextParams.delete(param)
without returning its result) to satisfy the Biome lint.
🧹 Nitpick comments (2)
src/hooks/usePrefixedSearchParams.ts (1)
90-99: Consider handling explicit deletions from the updater.When the updater explicitly deletes a key (rather than setting it to empty), the current logic won't remove the corresponding prefixed key from
nextParamssince the key won't appear inupdatedFiltered.entries(). The prefixed keys are already cleared on lines 70-74, so this works for prefixed params. However, ifuseGlobalParamsis true and the updater deletes a global key, that key won't be removed since it's only deleted if it appears with an empty value.This is a minor edge case that may not affect current usage but could cause confusion if someone expects
updaterdeletions to propagate for global keys.src/pages/views/components/ViewSection.tsx (1)
11-12: Guard against sections declaring both uiRef and viewRef.UIRef wiring looks good, but if both refs are present the viewRef path is silently ignored. Consider surfacing a configuration error to avoid ambiguous rendering.
🔧 Suggested guard
const isUIRefSection = !!section.uiRef; const isViewRefSection = !!section.viewRef; + if (isUIRefSection && isViewRefSection) { + return ( + <ErrorViewer + error="Invalid section: cannot set both viewRef and uiRef" + className="max-w-3xl" + /> + ); + }Also applies to: 27-29
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/ui/Dates/TimeRangePicker/useTimeRangeParams.tsx`:
- Around line 37-43: The code building defaultsParams misinterprets a
URLSearchParamsInit that is an array of tuples (ReadonlyArray<[string,string]>);
update the logic in useTimeRangeParams around the defaults -> defaultsParams
creation to explicitly detect when defaults is an array of key/value tuples
(Array.isArray(defaults) && defaults.every(e => Array.isArray(e) && e.length ===
2 && typeof e[0] === "string")) and pass that array directly to new
URLSearchParams, otherwise keep the existing handling for strings,
URLSearchParams instances, or object entries (and for object entries continue
flattening arrays of values as before); refer to the defaults variable and
defaultsParams construction in useTimeRangeParams.tsx and ensure
URLSearchParamsInit tuple arrays are handled before the Object.entries branch.
🧹 Nitpick comments (3)
src/ui/Dates/TimeRangePicker/useTimeRangeParams.tsx (3)
28-57: Consider documenting memoization requirement fordefaults.The
useEffectdependency ondefaultswill cause repeated effect executions if callers pass inline object literals (e.g.,useTimeRangeParams({ range: "1d" })). While thechangedcheck prevents unnecessary URL updates, the effect body still runs each render. Consider adding a JSDoc note advising callers to memoize thedefaultsobject or use a stable reference.📝 Suggested documentation update
/** * * useTimeRangeParams * * This hook is used to manage the time range parameters in the URL. It provides * a way to set the time range parameters and get the time range as an absolute * range, which can be consumed directly. It uses the useSearchParams hook from * react-router-dom to manage the URL parameters. * + * `@param` defaults - Initial URL params to apply if not already present. Should be + * memoized (useMemo) or defined outside the component to avoid repeated effect runs. + * `@param` paramPrefix - Optional prefix for namespacing URL parameters. */
106-114: Redundant optional chaining after null check.On line 112,
range?.toString()!uses optional chaining, but theif (range)check on line 108 already ensuresrangeis truthy. The?.and!are both redundant here.✨ Suggested simplification
if (rangeType === "relative") { const range = params.get("range"); if (range) { return { type: rangeType, display: display as string, - range: range?.toString()! + range: range } satisfies TimeRangeOption; } }
154-162: Inconsistent error handling for invalid mapped time range.
timeRangeValue(line 160) logs to console and implicitly returnsundefined, whiletimeRangeAbsoluteValue(line 207) throws an error for the same condition. This asymmetry could cause unexpected behavior if callers rely on one but not the other.If intentional (e.g.,
timeRangeValueis for display and can be lenient), consider adding a comment. Otherwise, align the error handling approach.Also applies to: 197-209
|
@adityathebe there seems to be a navigation loop on https://aws-preview-git-view-ui-refs-flanksource.vercel.app/views/b8d645ba-1581-4fde-a4cc-271f54512af7?view_mission-control_nodes__pageSize=10&view_mission-control_nodes__pageIndex=0&view_mission-control_nodes__sortBy=name&view_mission-control_nodes__sortOrder=asc Where the page loops and adds then removes |
adityathebe
force-pushed
the
view-ui-refs
branch
from
d8e8b45 to
3c329b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/api/query-hooks/useConfigChangesHooks.ts`:
- Around line 39-48: The function useGetAllConfigsChangesQuery currently loses
its default flags when callers pass an object with paramPrefix because the
parameter default only applies when no argument is provided; fix by merging
explicit defaults with the provided options: ensure useGetAllConfigsChangesQuery
accepts an options object (or default to {}), then combine defaultQueryOptions =
{ enabled: true, keepPreviousData: true } with the incoming options before
extracting paramPrefix and queryOptions so that queryOptions always contains
enabled and keepPreviousData unless explicitly overridden; update references to
paramPrefix and queryOptions accordingly.
In `@src/components/Forms/FormikFilterForm.tsx`:
- Around line 26-66: The two useEffect hooks (the one that calls setSearchParams
and the one that calls setFieldValue) create a feedback loop because the second
effect unconditionally calls setFieldValue; fix by adding a guard in the effect
that syncs URL -> form so it only calls setFieldValue when the incoming
searchParams value actually differs from the current Formik value
(values[field]) or when the params represent a genuine external change;
implement this by tracking the last-synced params in a ref (e.g.,
lastSyncedParamsRef) and comparing searchParams.get(field) against values[field]
(or lastSyncedParamsRef) before calling setFieldValue for each field (skip
calling setFieldValue when equal), so filterFields, searchParams, setFieldValue,
values and paramsToReset remain the reference points for the conditional logic.
In `@src/pages/views/components/ChangesUIFilters.tsx`:
- Around line 40-42: The forEach arrow callback implicitly returns the boolean
from nextParams.delete(param) which the linter flags; change the callback to a
block-bodied function so it does not return a value (e.g. replace
paramsToReset.configChanges.forEach((param) => nextParams.delete(param)) with a
block form that calls nextParams.delete(param) inside braces) or alternatively
iterate with a for...of loop over paramsToReset.configChanges and call
nextParams.delete(param) inside the loop; ensure you update the callback around
paramsToReset.configChanges.forEach and the call to nextParams.delete
accordingly.
- Around line 135-142: The current map over Object.entries(arbitraryFilters ??
{}) passes key={value} to FilterBadge which can cause duplicate React keys when
different param names share the same value; update the key to a composite unique
identifier (for example combine the entry key and value, e.g. `${key}-${value}`)
or include the index to guarantee uniqueness, keeping the props filters={value},
paramKey={key}, and paramPrefix={paramPrefix} unchanged so the component mapping
(FilterBadge) is stable.
In `@src/pages/views/components/ChangesUISection.tsx`:
- Around line 97-108: The code in the useMemo that builds changes uses non-null
assertions for change.type and change.name inside the returned config object
(the changes mapping in ChangesUISection, function/useMemo), which can propagate
undefined; update the mapping to avoid `!` by providing safe fallbacks (e.g.,
default strings or null) for type and name or derive reasonable values when
absent (use change.type ?? 'unknown' or change.name ?? ''), and ensure config.id
uses change.config_id safely as well; keep the same shape ({ id, type, name,
deleted_at }) so downstream rendering won't break when optional fields are
missing.
In `@src/ui/Dates/TimeRangePicker/useTimeRangeParams.tsx`:
- Around line 28-57: The useEffect in useTimeRangeParams calls setParams and can
cause URL churn when multiple instances run; update the effect to avoid
unnecessary param writes by comparing the serialized nextParams to currentParams
(e.g., compare nextParams.toString() vs currentParams.toString()) before calling
setParams, or when updating call setParams with a navigation option like
replace: true to avoid pushing history entries; ensure you reference the
existing variables nextParams, currentParams and the setParams updater in
useTimeRangeParams to apply the check and only return/perform the update when
the serialized outputs differ.
🧹 Nitpick comments (4)
src/pages/views/utils/__tests__/filterTranslation.test.ts (1)
123-153: Consider adding an idempotency test for already-translated values.The PR comments report a navigation loop where
health=healthy%3A-1(i.e.,healthy:-1) is repeatedly toggled in/out of the URL. This suggests that already-translated values may be getting re-translated. For example, ifhealthy:-1is fed back throughtranslateTristate, the-1portion could be misinterpreted.Adding a test case where the input is an already-translated value (e.g.,
translateTristate("healthy:-1")) would help confirm whether re-translation is the root cause:it("should be idempotent (not re-translate already translated values)", () => { // If "healthy:-1" is fed back in, the "-1" is treated as an exclusion of "1" // which would produce "healthy:1,1:-1" — confirming a double-translation bug const alreadyTranslated = "healthy:-1"; const result = translateTristate(alreadyTranslated); // Decide: should this be guarded or is double-translation prevented upstream? expect(result).toBe(/* expected value */); });This directly relates to the reported navigation loop bug.
src/pages/audit-report/types/index.ts (1)
466-471:ViewSectionallows bothviewRefanduiRefto be undefined simultaneously.With both fields optional, a section could have neither a view reference nor a UI reference, which seems semantically invalid. Consider enforcing at least one via a discriminated union or a runtime check.
Possible type-level enforcement
export type ViewSection = | { title: string; icon?: string; viewRef: ViewRef; uiRef?: undefined } | { title: string; icon?: string; viewRef?: undefined; uiRef: UIRef };src/components/Forms/FormikFilterForm.tsx (1)
102-113:initialValuesdepends onsearchParams, but Formik ignores post-mount changes toinitialValuesby default.The
useMemoincludessearchParamsin its dependency array, but<Formik initialValues={...}>only usesinitialValueson mount unlessenableReinitializeis set. This won't cause bugs but thesearchParamsdependency is misleading — it suggests reactivity that doesn't exist.src/pages/views/components/ViewSection.tsx (1)
68-82:sectionIdforviewRefsections uses unsanitized namespace/name.Line 73 constructs
sectionIdfrom rawsection.viewRef.namespaceandsection.viewRef.namewithout passing them throughtoSafeId. If these contain special characters, the resultingidattribute may be invalid. The other branches correctly usesafeTitle.Suggested fix
const sectionId = useMemo(() => { if (section.viewRef) { - return `section-${section.viewRef.namespace || "default"}-${section.viewRef.name}`; + return `section-${toSafeId(section.viewRef.namespace || "default")}-${toSafeId(section.viewRef.name || "")}`; }
| paramsToReset.configChanges.forEach((param) => | ||
| nextParams.delete(param) | ||
| ); |
There was a problem hiding this comment.
forEach callback should not return a value.
The arrow function implicitly returns the result of nextParams.delete(param), which the linter flags. Use a block body to suppress the warning.
Suggested fix
- paramsToReset.configChanges.forEach((param) =>
- nextParams.delete(param)
- );
+ paramsToReset.configChanges.forEach((param) => {
+ nextParams.delete(param);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| paramsToReset.configChanges.forEach((param) => | |
| nextParams.delete(param) | |
| ); | |
| paramsToReset.configChanges.forEach((param) => { | |
| nextParams.delete(param); | |
| }); |
🧰 Tools
🪛 Biome (2.3.14)
[error] 40-40: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@src/pages/views/components/ChangesUIFilters.tsx` around lines 40 - 42, The
forEach arrow callback implicitly returns the boolean from
nextParams.delete(param) which the linter flags; change the callback to a
block-bodied function so it does not return a value (e.g. replace
paramsToReset.configChanges.forEach((param) => nextParams.delete(param)) with a
block form that calls nextParams.delete(param) inside braces) or alternatively
iterate with a for...of loop over paramsToReset.configChanges and call
nextParams.delete(param) inside the loop; ensure you update the callback around
paramsToReset.configChanges.forEach and the call to nextParams.delete
accordingly.
| {Object.entries(arbitraryFilters ?? {}).map(([key, value]) => ( | ||
| <FilterBadge | ||
| filters={value} | ||
| key={value} | ||
| paramKey={key} | ||
| paramPrefix={paramPrefix} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
key={value} can collide when different param keys have the same value.
If two arbitrary filters have the same value string but different key names, React will see duplicate keys. Use a composite key instead.
Suggested fix
{Object.entries(arbitraryFilters ?? {}).map(([key, value]) => (
<FilterBadge
filters={value}
- key={value}
+ key={`${key}-${value}`}
paramKey={key}
paramPrefix={paramPrefix}
/>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {Object.entries(arbitraryFilters ?? {}).map(([key, value]) => ( | |
| <FilterBadge | |
| filters={value} | |
| key={value} | |
| paramKey={key} | |
| paramPrefix={paramPrefix} | |
| /> | |
| ))} | |
| {Object.entries(arbitraryFilters ?? {}).map(([key, value]) => ( | |
| <FilterBadge | |
| filters={value} | |
| key={`${key}-${value}`} | |
| paramKey={key} | |
| paramPrefix={paramPrefix} | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
In `@src/pages/views/components/ChangesUIFilters.tsx` around lines 135 - 142, The
current map over Object.entries(arbitraryFilters ?? {}) passes key={value} to
FilterBadge which can cause duplicate React keys when different param names
share the same value; update the key to a composite unique identifier (for
example combine the entry key and value, e.g. `${key}-${value}`) or include the
index to guarantee uniqueness, keeping the props filters={value},
paramKey={key}, and paramPrefix={paramPrefix} unchanged so the component mapping
(FilterBadge) is stable.
| const changes = useMemo(() => { | ||
| if (!data?.changes) return []; | ||
| return data.changes.map((change) => ({ | ||
| ...change, | ||
| config: { | ||
| id: change.config_id!, | ||
| type: change.type!, | ||
| name: change.name!, | ||
| deleted_at: change.deleted_at | ||
| } | ||
| })); | ||
| }, [data]); |
There was a problem hiding this comment.
Non-null assertions on optional fields type and name may cause runtime issues.
Per the ConfigChange type definition (src/api/types/configs.ts), type and name are optional (type?: string, name?: string). The ! assertions on lines 103-104 assume they're always present, which could propagate undefined into the config object and cause downstream rendering issues.
Suggested fix: provide safe fallbacks
const changes = useMemo(() => {
if (!data?.changes) return [];
return data.changes.map((change) => ({
...change,
config: {
id: change.config_id!,
- type: change.type!,
- name: change.name!,
+ type: change.type ?? "",
+ name: change.name ?? "",
deleted_at: change.deleted_at
}
}));
}, [data]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const changes = useMemo(() => { | |
| if (!data?.changes) return []; | |
| return data.changes.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id!, | |
| type: change.type!, | |
| name: change.name!, | |
| deleted_at: change.deleted_at | |
| } | |
| })); | |
| }, [data]); | |
| const changes = useMemo(() => { | |
| if (!data?.changes) return []; | |
| return data.changes.map((change) => ({ | |
| ...change, | |
| config: { | |
| id: change.config_id!, | |
| type: change.type ?? "", | |
| name: change.name ?? "", | |
| deleted_at: change.deleted_at | |
| } | |
| })); | |
| }, [data]); |
🤖 Prompt for AI Agents
In `@src/pages/views/components/ChangesUISection.tsx` around lines 97 - 108, The
code in the useMemo that builds changes uses non-null assertions for change.type
and change.name inside the returned config object (the changes mapping in
ChangesUISection, function/useMemo), which can propagate undefined; update the
mapping to avoid `!` by providing safe fallbacks (e.g., default strings or null)
for type and name or derive reasonable values when absent (use change.type ??
'unknown' or change.name ?? ''), and ensure config.id uses change.config_id
safely as well; keep the same shape ({ id, type, name, deleted_at }) so
downstream rendering won't break when optional fields are missing.
f216179 to
f03e761
Compare
View sections can now embed native Configs and Changes tables directly via a new uiRef field, instead of only referencing other views via viewRef. - Add ChangesUIFilters, ConfigsUIFilters, and UIRef types - Add ChangesUISection and ConfigsUISection components with preset filter seeding via URL params - Add ConfigsUIFilters and ChangesUIFilters filter bar components - Add filter translation utilities to convert backend formats (Kubernetes- style selectors) to UI internal tristate format - Update ViewSection to branch rendering based on viewRef vs uiRef - Update ViewWithSections key generation for uiRef sections - Skip uiRef sections in useViewData view fetching
adityathebe
force-pushed
the
view-ui-refs
branch
from
f03e761 to
604922c
Compare
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests