OCPBUGS-44612: Remove useUserPreferenceCompatibility and withUserPreferenceCompatibility#16076
OCPBUGS-44612: Remove useUserPreferenceCompatibility and withUserPreferenceCompatibility#16076vojtechszocs wants to merge 2 commits intoopenshift:mainfrom
Conversation
The hook handles a single user preference value, so rename it from useUserSettings to useUserPreference for clarity and consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…lity Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
openshift-ci-robot
added
jira/valid-reference
|
@vojtechszocs: This pull request references Jira Issue OCPBUGS-44612, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
component/core
| import { useUserPreferenceCompatibility } from '@console/shared/src/hooks/useUserPreferenceCompatibility'; | ||
| import { FAVORITES_CONFIG_MAP_KEY, FAVORITES_LOCAL_STORAGE_KEY } from '../../consts'; | ||
| import { useUserPreference } from '@console/shared/src/hooks/useUserPreference'; | ||
| import { FAVORITES_CONFIG_MAP_KEY } from '../../consts'; |
There was a problem hiding this comment.
should we be consistently calling these user_settings_key or even user_preference_key?
There was a problem hiding this comment.
This PR is meant to remove/replace existing legacy code but I'm OK with adding more improvements.
- remove unused
*_LOCAL_STORAGE_KEYconstants (formeruseUserPreferenceCompatibilityhookstorageKeyparameter) - apply consistent naming to user preference key constants (currently a mix of
*_USER_SETTINGS_KEY,*_USERSETTINGS_KEY,*_CONFIG_MAP_KEY,*_CONFIGMAP_KEYetc.)
📝 WalkthroughWalkthroughThis pull request refactors the user preferences system across the codebase. The deprecated 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/packages/dev-console/src/components/buildconfig/__tests__/BuildConfigFormPage.spec.tsx (1)
19-33:⚠️ Potential issue | 🟡 MinorDuplicate mock declarations — first mock is dead code.
Both mocks target
@console/shared/src/hooks/useUserPreference. Jest uses the last mock declaration, so lines 19-21 are never applied. The "For internal used Dropdowns" comment suggests these were originally two different hooks being mocked (likelyuseUserSettingsCompatibilityanduseUserSettings), but after the migration they now reference the same module.Additionally, the static mock at line 20 returns only 2 elements
['', () => {}], butuseUserPreferencereturns a 3-element tuple[value, setter, loaded].Consider removing the redundant mock block:
🧹 Proposed fix to remove dead code
-// For internal used Dropdowns -jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ - useUserPreference: () => ['', () => {}], -})); - jest.mock('@console/shared/src/hooks/useResizeObserver', () => ({ useResizeObserver: () => {}, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/dev-console/src/components/buildconfig/__tests__/BuildConfigFormPage.spec.tsx` around lines 19 - 33, Remove the dead/duplicate mock for useUserPreference (the static jest.mock that returns ['', () => {}]) and keep a single mock declaration for the module (the jest.fn() one); also ensure the retained mock for useUserPreference returns the correct 3-tuple shape [value, setter, loaded] whenever referenced in tests (e.g., adjust the mock implementation used by tests that reference useUserPreference to return [<string>, <jest.fn()>, <boolean>]). Locate the mocks by the module name useUserPreference and the test file's mocked symbols to update or delete the redundant block and fix the tuple shape.frontend/packages/console-shared/src/hooks/useUserPreference.ts (1)
107-115:⚠️ Potential issue | 🟠 MajorPotential legacy preference loss after compatibility removal
Line 107 now only reads the consolidated
console-user-settings*payload path. Users who still only have legacy per-keylocalStorageentries (never logged in during migration window) will not get migrated and can silently lose saved preferences.Please add a one-time legacy fallback read (or explicit migration step) before returning defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-shared/src/hooks/useUserPreference.ts` around lines 107 - 115, Before returning defaults in useUserPreference, add a one-time legacy fallback read/migration that checks window.localStorage for the old per-key entry (using keyRef.current) and, if found and the consolidated payload (the value returned by useUserPreferenceLocalStorage / lsData) is empty, migrate that value into the consolidated store via setLsDataCallback and remove the legacy key; ensure the migration respects impersonate/alwaysUseFallbackLocalStorage and uses defaultValueRef.current only if neither legacy nor consolidated data exist.frontend/packages/topology/src/components/side-bar/components/SideBarAlerts.tsx (1)
24-30:⚠️ Potential issue | 🟡 MinorFalsy coalescing here can override a legitimate
falsevalue.On Line 29,
|| truewill coerce a storedfalsetotrue. Use nullish coalescing so onlynull/undefinedfall back to default visibility.Proposed fix
const [showAlert, setShowAlert] = useUserPreferenceLocalStorage( `${element.getId()}`, `${USERSETTINGS_PREFIX}/${SIDEBAR_ALERTS}/${id}`, deserializeData( cfData?.data?.[`${USERSETTINGS_PREFIX}.${SIDEBAR_ALERTS}.${id}.${element.getId()}`], - ) || true, + ) ?? true, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/topology/src/components/side-bar/components/SideBarAlerts.tsx` around lines 24 - 30, The fallback for the initial showAlert value currently uses `|| true`, which will turn a stored false into true; update the initialization in the useUserPreferenceLocalStorage call (where showAlert/setShowAlert are created) to use nullish coalescing instead (replace `|| true` with `?? true`) so only null or undefined fall back to the default after calling `deserializeData` on `cfData?.data?...`.frontend/packages/console-app/src/components/user-preferences/UserPreferenceCheckboxField.tsx (1)
34-36:⚠️ Potential issue | 🟡 MinorPotential bug: falsy preference values may be incorrectly overwritten.
The condition
!currentUserPreferenceValuewill be true for any falsy value includingfalse,0, or''. If a user intentionally sets their checkbox preference tofalse, this logic will overwrite it with thedefaultValue.Consider checking for
undefinedexplicitly, or passingdefaultValuedirectly touseUserPreference:🐛 Proposed fix
Option 1: Pass defaultValue to the hook (preferred)
const [ currentUserPreferenceValue, setCurrentUserPreferenceValue, currentUserPreferenceValueLoaded, - ] = useUserPreference<UserPreferenceCheckboxFieldValue>(userSettingsKey); + ] = useUserPreference<UserPreferenceCheckboxFieldValue>(userSettingsKey, defaultValue); const fireTelemetryEvent = useTelemetry(); const loaded: boolean = currentUserPreferenceValueLoaded; - if (defaultValue && loaded && !currentUserPreferenceValue) { - setCurrentUserPreferenceValue(defaultValue); - }Option 2: Check for undefined explicitly
- if (defaultValue && loaded && !currentUserPreferenceValue) { + if (defaultValue !== undefined && loaded && currentUserPreferenceValue === undefined) { setCurrentUserPreferenceValue(defaultValue); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/user-preferences/UserPreferenceCheckboxField.tsx` around lines 34 - 36, The current check in UserPreferenceCheckboxField.tsx uses "!currentUserPreferenceValue" which treats false/0/'' as unset and will overwrite explicit falsy preferences; change this to check for undefined explicitly (e.g., currentUserPreferenceValue === undefined) before calling setCurrentUserPreferenceValue, or better, pass defaultValue into the useUserPreference hook so the hook initializes the preference correctly and you can remove this overwrite logic; update references to defaultValue, loaded, currentUserPreferenceValue, and setCurrentUserPreferenceValue accordingly.frontend/packages/topology/src/components/page/TopologyPage.tsx (1)
28-32:⚠️ Potential issue | 🔴 CriticalRemove
activeViewStorageKeyprop and update test calls—it's no longer used after theuseUserPreferencemigration.The
activeViewStorageKeyprop is declared in the interface and destructured but never used in the component implementation. However, the test suite still passes this prop in three test cases (TopologyPage.spec.tsxlines 116, 123, 131), so the cleanup requires updating both the component and tests.♻️ Required changes
Component changes (TopologyPage.tsx):
interface TopologyPageProps { - activeViewStorageKey?: string; hideProjects?: boolean; defaultViewType?: TopologyViewType; }export const TopologyPage: FC<TopologyPageProps> = ({ - activeViewStorageKey = LAST_TOPOLOGY_VIEW_LOCAL_STORAGE_KEY, hideProjects = false, defaultViewType = TopologyViewType.graph, }) => {Test updates (TopologyPage.spec.tsx):
RemoveactiveViewStorageKey="fake-key"prop from the three test renders at lines 116, 123, and 131.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/topology/src/components/page/TopologyPage.tsx` around lines 28 - 32, The TopologyPage component still declares and destructures the now-unused prop activeViewStorageKey in the TopologyPageProps interface and component signature; remove activeViewStorageKey from the TopologyPageProps interface and from the destructured props in the TopologyPage component (search for TopologyPageProps and TopologyPage function/component). Then update the tests in TopologyPage.spec.tsx by removing the activeViewStorageKey="fake-key" prop from the three renders that pass it so the tests match the new component signature. Ensure no other references to activeViewStorageKey remain.
🧹 Nitpick comments (8)
frontend/public/components/global-telemetry-notifications.tsx (1)
14-17: Hook migration looks correct; consider using theloadedstate.The hook invocation is correct: key, default value (
null), andsync: trueall match the expecteduseUserPreferencesignature.One minor observation: the hook returns a third value (
loaded) that isn't destructured here. Currently,showTelemetryNotificationrelies on!currentUserPreferenceTelemetryValue, which would be truthy while the preference is still loading (since it starts asnull/undefined). This could cause a brief flash of the banner for users who have already responded, until the ConfigMap value loads.If this is intentional or acceptable UX, no action needed. Otherwise, you could gate on
loadedto prevent the flash:♻️ Optional: Gate on loaded state to prevent flash
const [ currentUserPreferenceTelemetryValue, setCurrentUserPreferenceTelemetryValue, - ] = useUserPreference<USER_TELEMETRY_ANALYTICS>(PREFERRED_TELEMETRY_USER_SETTING_KEY, null, true); + loaded, + ] = useUserPreference<USER_TELEMETRY_ANALYTICS>(PREFERRED_TELEMETRY_USER_SETTING_KEY, null, true);Then update the condition:
const showTelemetryNotification = + loaded && !currentUserPreferenceTelemetryValue && (window.SERVER_FLAGS.telemetry?.STATE === CLUSTER_TELEMETRY_ANALYTICS.OPTIN || window.SERVER_FLAGS.telemetry?.STATE === CLUSTER_TELEMETRY_ANALYTICS.OPTOUT);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/global-telemetry-notifications.tsx` around lines 14 - 17, The hook useUserPreference returns a third "loaded" flag that's not being used, causing showTelemetryNotification to rely on currentUserPreferenceTelemetryValue which may be null while the preference is still loading and cause a flash; update the destructuring to include the loaded value (e.g., const [currentUserPreferenceTelemetryValue, setCurrentUserPreferenceTelemetryValue, loaded] = useUserPreference(...)) and then change the condition that computes showTelemetryNotification to require loaded (e.g., showTelemetryNotification only when loaded is true and currentUserPreferenceTelemetryValue indicates no preference) so the banner is not shown until the preference has finished loading.frontend/public/components/__tests__/markdown-view.spec.tsx (1)
18-22: Mock from the public API surface, not internal paths, to prevent brittle Jest wiring.Your test imports
useUserPreferencefrom@console/shared(line 4), but mocks the deep internal path@console/shared/src/hooks/useUserPreference(lines 18–20). SinceuseUserPreferenceis already publicly re-exported from@console/shared, use a partial mock instead so the test remains stable if internal re-exports are refactored.Suggested refactor
-jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ - useUserPreference: jest.fn(), -})); +jest.mock('@console/shared', () => ({ + ...jest.requireActual('@console/shared'), + useUserPreference: jest.fn(), +})); -const mockUserPreference = useUserPreference as jest.Mock; +const mockUserPreference = useUserPreference as jest.MockedFunction<typeof useUserPreference>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/__tests__/markdown-view.spec.tsx` around lines 18 - 22, Summary: The test currently mocks the internal module path '@console/shared/src/hooks/useUserPreference' instead of the public API, which makes the Jest wiring brittle; change it to partially mock the public '@console/shared' export and keep other exports intact. Replace the existing jest.mock call that targets '@console/shared/src/hooks/useUserPreference' with a partial mock of '@console/shared' that spreads jest.requireActual('@console/shared') and overrides useUserPreference with jest.fn(), and keep the test's mock reference (mockUserPreference = useUserPreference as jest.Mock) so it points to the public re-export; update any import lines that reference useUserPreference to import from '@console/shared' if not already.frontend/packages/helm-plugin/src/topology/__tests__/TopologyHelmReleaseNotesPanel.spec.tsx (1)
2-16: Consider importinguseUserPreferencefrom the same module path you mock.
useUserPreferenceis imported from the public barrel export@console/shared, but mocked directly from@console/shared/src/hooks/useUserPreference. While this works correctly—Jest intercepts the source module before re-exports—aligning both paths improves consistency. Either import from@console/shared/src/hooks/useUserPreferenceor mock from the public path for stylistic clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/helm-plugin/src/topology/__tests__/TopologyHelmReleaseNotesPanel.spec.tsx` around lines 2 - 16, The import path for useUserPreference in TopologyHelmReleaseNotesPanel.spec.tsx is inconsistent with the module path used in the jest.mock; update either the import or the mock so both reference the same module path (e.g., import useUserPreference from '@console/shared/src/hooks/useUserPreference' to match the existing jest.mock, or change the jest.mock to '@console/shared' to match the current import); adjust the mock alias (mockUserPreference = useUserPreference as jest.Mock) accordingly and ensure the mocked symbol name and jest.mock target refer to the identical module path.frontend/public/components/dashboard/project-dashboard/getting-started/__tests__/GettingStartedSection.spec.tsx (1)
4-4: Mock the specific hook instead of the entire@console/shared/srcdirectory for consistency.Line 4 imports
useUserPreferencefrom@console/shared, but lines 27-30 mock the entire@console/shared/srcdirectory. The sibling test filegetting-started-section.spec.tsxuses a more specific and maintainable pattern by mocking the hook directly. Align this file to match:Proposed fix
-jest.mock('@console/shared/src', () => ({ - ...jest.requireActual('@console/shared/src'), +jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ useUserPreference: jest.fn(() => [true, jest.fn()]), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/dashboard/project-dashboard/getting-started/__tests__/GettingStartedSection.spec.tsx` at line 4, The test currently mocks the entire '@console/shared/src' module; instead, update the test to mock only the useUserPreference hook so it's consistent and maintainable: replace the broad mock with a specific jest.mock for '@console/shared' that stubs useUserPreference (the hook imported as useUserPreference) and return the desired value(s) in the test setup; ensure any existing references to the previous mock are removed and tests import useUserPreference from '@console/shared' as in the file's import.frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts (1)
798-802: Maintain type-level backward compatibility for the renamed export.A deprecated function alias (
useUserSettings) exists indynamic-core-api.ts, which is good for plugins using the hook directly. However, the typeUseUserSettings<T>has been completely removed fromconsole-types.ts. External plugins that import this type for their own type definitions will break without an explicit type alias.Add the deprecated type alias to
console-types.tsalongside the function alias for complete backward compatibility across both usage patterns:Suggested addition
/** `@deprecated` Use UseUserPreference instead */ export type UseUserSettings<T> = UseUserPreference<T>;This ensures plugins can migrate incrementally rather than all at once.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts` around lines 798 - 802, The exported hook type UseUserPreference was renamed but the deprecated type alias UseUserSettings is missing, breaking downstream type imports; to fix, add a deprecated type alias that maps UseUserSettings<T> to UseUserPreference<T> (e.g., add a line declaring "export type UseUserSettings<T> = UseUserPreference<T>;" with a `@deprecated` JSDoc) so both the new UseUserPreference type and the old UseUserSettings name coexist for backward compatibility.frontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsx (1)
48-51: Missing type parameter and loading state handling.Two observations:
useAllQuickStartStates(line 51) lacks an explicit type parameter. Consider adding one for clarity and type safety.Neither hook destructures the
loadedboolean (third element). The quick start states are used immediately in callbacks without confirming data has loaded. This was likely the same in the previous implementation, but worth noting for consistency with other migrations in this PR.♻️ Optional: Add explicit type for clarity
+type AllQuickStartStates = Record<string, unknown>; + const useActiveQuickStartId = () => useUserPreference<string>(ACTIVE_QUICK_START_ID_KEY, getInitialState()?.activeQuickStartId ?? ''); const useAllQuickStartStates = () => - useUserPreference(ALL_QUICK_START_STATE_KEY, getInitialState()?.allQuickStartStates ?? {}); + useUserPreference<AllQuickStartStates>(ALL_QUICK_START_STATE_KEY, getInitialState()?.allQuickStartStates ?? {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsx` around lines 48 - 51, useAllQuickStartStates is missing an explicit generic type and both hooks (useActiveQuickStartId and useAllQuickStartStates) fail to destructure the third returned "loaded" value from useUserPreference; update useAllQuickStartStates to call useUserPreference<Record<string, QuickStartState>>(...) (or the appropriate type used elsewhere) and change both hooks to destructure [value, setValue, loaded] = useUserPreference<...>(...) and return or check the loaded flag where these hooks are consumed so callbacks using quick start state only run after loaded is true (or guard early with the loaded boolean inside the hook consumers such as in any callback that reads the state).frontend/packages/console-dynamic-plugin-sdk/src/api/dynamic-core-api.ts (1)
936-939: Consider adding a target removal release to the deprecation note.Including an expected removal milestone/version improves upgrade planning for plugin maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/src/api/dynamic-core-api.ts` around lines 936 - 939, Update the deprecation JSDoc for useUserSettings to include a target removal release/version and migration hint: modify the comment above the export of useUserSettings (symbol useUserSettings) to say it's deprecated in favor of useUserPreference (symbol useUserPreference) and include the expected removal milestone (e.g., "Removed in vX.Y.0") plus a short migration note to use useUserPreference instead so plugin maintainers can plan upgrades.frontend/packages/console-dynamic-plugin-sdk/docs/api.md (1)
3141-3148: Minor: Example variable names could better reflect the new terminology.The example uses
userPreferenceStateandsetUserPreferenceState, which is good, but consider using more concise names likepreference/setPreferenceto match typical React conventions and the hook's simplified API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/docs/api.md` around lines 3141 - 3148, Rename the example variables to be more concise: replace userPreferenceState and setUserPreferenceState with preference and setPreference respectively where useUserPreference is called and where props are passed into WrappedComponent; update the prop name passed (e.g., userPreferenceState={preference} setUserPreferenceState={setPreference}) and any corresponding prop references in the example to match the new names so the sample reflects the hook's simplified API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/packages/console-app/src/components/user-preferences/search/useExactSearch.ts`:
- Around line 6-9: The hook useExactSearch currently returns a boolean but calls
useUserPreference<boolean>(PREFERRED_EXACT_SEARCH_USER_SETTING_KEY) without a
default, which can yield undefined on first render; update the call in
useExactSearch to pass a safe default (false) to useUserPreference so
exactSearch is always a boolean, and keep returning [exactSearch,
exactSearchLoaded] from useExactSearch unchanged (useUserPreference must be
invoked with the default false for PREFERRED_EXACT_SEARCH_USER_SETTING_KEY).
In
`@frontend/packages/console-app/src/components/user-preferences/synced-editor/usePreferredCreateEditMethod.ts`:
- Around line 7-9: The hook can return undefined because useUserPreference is
called without a default; update the call to useUserPreference in
usePreferredCreateEditMethod (the line that sets preferredCreateEditMethod using
PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY) to pass an explicit default
string (e.g., '') as the second argument so the hook always returns a string and
satisfies the declared [string, boolean] contract.
In
`@frontend/packages/console-shared/src/components/dropdown/__tests__/ResourceDropdown.spec.tsx`:
- Around line 6-9: The mock for useUserPreference should match the real hook
signature [value, setter, loaded]; update the jest.mock in
ResourceDropdown.spec.tsx so the mocked useUserPreference returns a 3-tuple
(e.g. ['', () => {}, true]) instead of two items, ensuring the loaded flag is
present; locate the mock definition for useUserPreference and adjust its return
to include the third boolean element.
In
`@frontend/packages/console-shared/src/components/namespace/NamespaceDropdown.tsx`:
- Around line 230-236: The current replacement of legacy migration with direct
useUserPreference (keys favoritesUserPreferenceKey and
systemNamespacesSettingsKey) can drop dormant-user settings; add a one-time
migration fallback: when useUserPreference returns undefined for favorites or
system namespace, read the legacy localStorage keys (the old
bookmarks/system-namespace keys), validate/parse them, then call the
corresponding setter (setFavorites or the system namespace setter used near
lines referencing systemNamespacesSettingsKey) to persist the migrated value and
optionally mark a migrated flag so this runs only once; alternatively,
explicitly document/emit a telemetry/upgrade flag if you intend to accept loss
of legacy data.
In `@frontend/packages/console-shared/src/hoc/withUserPreference.tsx`:
- Around line 4-7: Update the WithUserPreferenceProps generic to type the setter
as React's Dispatch<SetStateAction<T>> instead of (v: T) => void so consumers
can pass functional updaters; specifically change the setUserSettingState
declaration in WithUserPreferenceProps to use Dispatch<SetStateAction<T>>
(matching useUserPreference's return type and internal handling) so components
like ColumnManagementModal can call setTableColumns(prev => ...) without type
errors.
In
`@frontend/packages/helm-plugin/src/components/forms/__tests__/HelmInstallUpgradeForm.spec.tsx`:
- Around line 54-56: The mock for useUserPreference in the test returns only a
two-element tuple which leaves the third `loaded` value undefined and can change
render branches; update the jest.mock for
'@console/shared/src/hooks/useUserPreference' so useUserPreference returns the
full three-element tuple (e.g. [value, setter, loaded])—set the third element to
an explicit boolean (true or false) appropriate for the test scenario—to ensure
component rendering paths are deterministic.
In `@frontend/packages/topology/src/__tests__/TopologyPage.spec.tsx`:
- Around line 97-103: Mocks for useUserPreference and usePreferredTopologyView
in TopologyPage.spec.tsx are returning only two items and must include the third
`loaded` boolean; update the mocked returns (e.g., change (useUserPreference as
jest.Mock).mockReturnValue(['', () => {}]) to mockReturnValue(['', () => {},
true]) and change (usePreferredTopologyView as jest.Mock).mockReturnValue(['',
true]) to mockReturnValue(['', true, true]) (and apply the same 3-element tuple
fix for the other test block mentioned) so the component sees the full [value,
setter, loaded] tuple during tests.
In `@frontend/packages/topology/src/filters/FilterProvider.tsx`:
- Around line 40-43: The current useUserPreference call for
TOPOLOGY_DISPLAY_FILTERS_USER_SETTINGS_KEY loses any legacy localStorage-stored
filters; add a one-time fallback migration: keep the
useUserPreference([appliedFilters, setAppliedFilters, appliedFiltersLoaded])
call, then in a useEffect that runs when appliedFiltersLoaded becomes true and
appliedFilters equals getAppliedFilters(DEFAULT_TOPOLOGY_FILTERS), read the
legacy storage key (e.g., LEGACY_TOPOLOGY_FILTERS_KEY) from localStorage, parse
and validate it via getAppliedFilters, call setAppliedFilters(migratedValue) if
valid, and then remove the legacy key so the migration runs only once; reference
appliedFilters, setAppliedFilters, appliedFiltersLoaded, useUserPreference,
TOPOLOGY_DISPLAY_FILTERS_USER_SETTINGS_KEY, getAppliedFilters, and
DEFAULT_TOPOLOGY_FILTERS in the change.
In
`@frontend/public/components/dashboard/project-dashboard/getting-started/GettingStartedSection.tsx`:
- Around line 24-27: The call to useUserPreference can briefly return undefined
while loading, causing isGettingStartedSectionOpen (from useUserPreference) to
be undefined when consumers expect a boolean; update the initialization in
GettingStartedSection.tsx to guard against transient undefined by using a safe
fallback (e.g., coalesce isGettingStartedSectionOpen with true via `?? true`) or
gate rendering/behavior on the preference's loaded flag returned by
useUserPreference; update references to isGettingStartedSectionOpen and
setIsGettingStartedSectionOpen accordingly so consumers always see a boolean.
In `@frontend/public/components/utils/console-select.tsx`:
- Around line 147-157: The code calls useUserPreference with
bookmarkUserPreferenceKey which can be undefined; either stop calling
useUserPreference when bookmarkUserPreferenceKey is undefined (wrap the
useUserPreference call behind the existing enableBookmarks check so bookmarks
and setBookmarks are only created when enableBookmarks is true), or update the
useUserPreference implementation to early-return a no-op pair (empty state and a
setter that does nothing) whenever its key parameter is undefined (ensure
keyRef.current and any cfData access and updateConfigMap calls are guarded
against undefined). Reference: bookmarkUserPreferenceKey, enableBookmarks,
useUserPreference, keyRef.current, cfData, updateConfigMap.
---
Outside diff comments:
In
`@frontend/packages/console-app/src/components/user-preferences/UserPreferenceCheckboxField.tsx`:
- Around line 34-36: The current check in UserPreferenceCheckboxField.tsx uses
"!currentUserPreferenceValue" which treats false/0/'' as unset and will
overwrite explicit falsy preferences; change this to check for undefined
explicitly (e.g., currentUserPreferenceValue === undefined) before calling
setCurrentUserPreferenceValue, or better, pass defaultValue into the
useUserPreference hook so the hook initializes the preference correctly and you
can remove this overwrite logic; update references to defaultValue, loaded,
currentUserPreferenceValue, and setCurrentUserPreferenceValue accordingly.
In `@frontend/packages/console-shared/src/hooks/useUserPreference.ts`:
- Around line 107-115: Before returning defaults in useUserPreference, add a
one-time legacy fallback read/migration that checks window.localStorage for the
old per-key entry (using keyRef.current) and, if found and the consolidated
payload (the value returned by useUserPreferenceLocalStorage / lsData) is empty,
migrate that value into the consolidated store via setLsDataCallback and remove
the legacy key; ensure the migration respects
impersonate/alwaysUseFallbackLocalStorage and uses defaultValueRef.current only
if neither legacy nor consolidated data exist.
In
`@frontend/packages/dev-console/src/components/buildconfig/__tests__/BuildConfigFormPage.spec.tsx`:
- Around line 19-33: Remove the dead/duplicate mock for useUserPreference (the
static jest.mock that returns ['', () => {}]) and keep a single mock declaration
for the module (the jest.fn() one); also ensure the retained mock for
useUserPreference returns the correct 3-tuple shape [value, setter, loaded]
whenever referenced in tests (e.g., adjust the mock implementation used by tests
that reference useUserPreference to return [<string>, <jest.fn()>, <boolean>]).
Locate the mocks by the module name useUserPreference and the test file's mocked
symbols to update or delete the redundant block and fix the tuple shape.
In `@frontend/packages/topology/src/components/page/TopologyPage.tsx`:
- Around line 28-32: The TopologyPage component still declares and destructures
the now-unused prop activeViewStorageKey in the TopologyPageProps interface and
component signature; remove activeViewStorageKey from the TopologyPageProps
interface and from the destructured props in the TopologyPage component (search
for TopologyPageProps and TopologyPage function/component). Then update the
tests in TopologyPage.spec.tsx by removing the activeViewStorageKey="fake-key"
prop from the three renders that pass it so the tests match the new component
signature. Ensure no other references to activeViewStorageKey remain.
In
`@frontend/packages/topology/src/components/side-bar/components/SideBarAlerts.tsx`:
- Around line 24-30: The fallback for the initial showAlert value currently uses
`|| true`, which will turn a stored false into true; update the initialization
in the useUserPreferenceLocalStorage call (where showAlert/setShowAlert are
created) to use nullish coalescing instead (replace `|| true` with `?? true`) so
only null or undefined fall back to the default after calling `deserializeData`
on `cfData?.data?...`.
---
Nitpick comments:
In
`@frontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsx`:
- Around line 48-51: useAllQuickStartStates is missing an explicit generic type
and both hooks (useActiveQuickStartId and useAllQuickStartStates) fail to
destructure the third returned "loaded" value from useUserPreference; update
useAllQuickStartStates to call useUserPreference<Record<string,
QuickStartState>>(...) (or the appropriate type used elsewhere) and change both
hooks to destructure [value, setValue, loaded] = useUserPreference<...>(...) and
return or check the loaded flag where these hooks are consumed so callbacks
using quick start state only run after loaded is true (or guard early with the
loaded boolean inside the hook consumers such as in any callback that reads the
state).
In `@frontend/packages/console-dynamic-plugin-sdk/docs/api.md`:
- Around line 3141-3148: Rename the example variables to be more concise:
replace userPreferenceState and setUserPreferenceState with preference and
setPreference respectively where useUserPreference is called and where props are
passed into WrappedComponent; update the prop name passed (e.g.,
userPreferenceState={preference} setUserPreferenceState={setPreference}) and any
corresponding prop references in the example to match the new names so the
sample reflects the hook's simplified API.
In `@frontend/packages/console-dynamic-plugin-sdk/src/api/dynamic-core-api.ts`:
- Around line 936-939: Update the deprecation JSDoc for useUserSettings to
include a target removal release/version and migration hint: modify the comment
above the export of useUserSettings (symbol useUserSettings) to say it's
deprecated in favor of useUserPreference (symbol useUserPreference) and include
the expected removal milestone (e.g., "Removed in vX.Y.0") plus a short
migration note to use useUserPreference instead so plugin maintainers can plan
upgrades.
In
`@frontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.ts`:
- Around line 798-802: The exported hook type UseUserPreference was renamed but
the deprecated type alias UseUserSettings is missing, breaking downstream type
imports; to fix, add a deprecated type alias that maps UseUserSettings<T> to
UseUserPreference<T> (e.g., add a line declaring "export type UseUserSettings<T>
= UseUserPreference<T>;" with a `@deprecated` JSDoc) so both the new
UseUserPreference type and the old UseUserSettings name coexist for backward
compatibility.
In
`@frontend/packages/helm-plugin/src/topology/__tests__/TopologyHelmReleaseNotesPanel.spec.tsx`:
- Around line 2-16: The import path for useUserPreference in
TopologyHelmReleaseNotesPanel.spec.tsx is inconsistent with the module path used
in the jest.mock; update either the import or the mock so both reference the
same module path (e.g., import useUserPreference from
'@console/shared/src/hooks/useUserPreference' to match the existing jest.mock,
or change the jest.mock to '@console/shared' to match the current import);
adjust the mock alias (mockUserPreference = useUserPreference as jest.Mock)
accordingly and ensure the mocked symbol name and jest.mock target refer to the
identical module path.
In `@frontend/public/components/__tests__/markdown-view.spec.tsx`:
- Around line 18-22: Summary: The test currently mocks the internal module path
'@console/shared/src/hooks/useUserPreference' instead of the public API, which
makes the Jest wiring brittle; change it to partially mock the public
'@console/shared' export and keep other exports intact. Replace the existing
jest.mock call that targets '@console/shared/src/hooks/useUserPreference' with a
partial mock of '@console/shared' that spreads
jest.requireActual('@console/shared') and overrides useUserPreference with
jest.fn(), and keep the test's mock reference (mockUserPreference =
useUserPreference as jest.Mock) so it points to the public re-export; update any
import lines that reference useUserPreference to import from '@console/shared'
if not already.
In
`@frontend/public/components/dashboard/project-dashboard/getting-started/__tests__/GettingStartedSection.spec.tsx`:
- Line 4: The test currently mocks the entire '@console/shared/src' module;
instead, update the test to mock only the useUserPreference hook so it's
consistent and maintainable: replace the broad mock with a specific jest.mock
for '@console/shared' that stubs useUserPreference (the hook imported as
useUserPreference) and return the desired value(s) in the test setup; ensure any
existing references to the previous mock are removed and tests import
useUserPreference from '@console/shared' as in the file's import.
In `@frontend/public/components/global-telemetry-notifications.tsx`:
- Around line 14-17: The hook useUserPreference returns a third "loaded" flag
that's not being used, causing showTelemetryNotification to rely on
currentUserPreferenceTelemetryValue which may be null while the preference is
still loading and cause a flash; update the destructuring to include the loaded
value (e.g., const [currentUserPreferenceTelemetryValue,
setCurrentUserPreferenceTelemetryValue, loaded] = useUserPreference(...)) and
then change the condition that computes showTelemetryNotification to require
loaded (e.g., showTelemetryNotification only when loaded is true and
currentUserPreferenceTelemetryValue indicates no preference) so the banner is
not shown until the preference has finished loading.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (95)
frontend/packages/console-app/src/components/detect-namespace/useLastNamespace.tsfrontend/packages/console-app/src/components/detect-perspective/useLastPerspective.tsfrontend/packages/console-app/src/components/favorite/FavoriteButton.tsxfrontend/packages/console-app/src/components/favorite/FavoriteNavItems.tsxfrontend/packages/console-app/src/components/nodes/NodeLogs.tsxfrontend/packages/console-app/src/components/nodes/NodesPage.tsxfrontend/packages/console-app/src/components/quick-starts/utils/quick-start-context.tsxfrontend/packages/console-app/src/components/tour/__tests__/tour-context.spec.tsfrontend/packages/console-app/src/components/tour/tour-context.tsfrontend/packages/console-app/src/components/user-preferences/UserPreferenceCheckboxField.tsxfrontend/packages/console-app/src/components/user-preferences/UserPreferenceDropdownField.tsxfrontend/packages/console-app/src/components/user-preferences/__tests__/UserPreferenceCheckboxField.spec.tsxfrontend/packages/console-app/src/components/user-preferences/__tests__/UserPreferenceDropdownField.spec.tsxfrontend/packages/console-app/src/components/user-preferences/language/usePreferredLanguage.tsfrontend/packages/console-app/src/components/user-preferences/lightspeed/useHideLightspeed.tsfrontend/packages/console-app/src/components/user-preferences/namespace/usePreferredNamespace.tsfrontend/packages/console-app/src/components/user-preferences/perspective/usePreferredPerspective.tsfrontend/packages/console-app/src/components/user-preferences/search/useExactSearch.tsfrontend/packages/console-app/src/components/user-preferences/synced-editor/usePreferredCreateEditMethod.tsfrontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.mdfrontend/packages/console-dynamic-plugin-sdk/docs/api.mdfrontend/packages/console-dynamic-plugin-sdk/src/api/dynamic-core-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-shared/src/components/dropdown/ResourceDropdown.tsxfrontend/packages/console-shared/src/components/dropdown/__tests__/ResourceDropdown.spec.tsxfrontend/packages/console-shared/src/components/getting-started/useGettingStartedShowState.tsxfrontend/packages/console-shared/src/components/health-checks/HealthChecksAlert.tsxfrontend/packages/console-shared/src/components/health-checks/__tests__/HealthChecksAlert.spec.tsxfrontend/packages/console-shared/src/components/namespace/NamespaceDropdown.tsxfrontend/packages/console-shared/src/components/synced-editor/__tests__/useEditorType.spec.tsfrontend/packages/console-shared/src/components/synced-editor/useEditorType.tsfrontend/packages/console-shared/src/hoc/index.tsfrontend/packages/console-shared/src/hoc/withUserPreference.tsxfrontend/packages/console-shared/src/hoc/withUserSettings.tsxfrontend/packages/console-shared/src/hoc/withUserSettingsCompatibility.tsxfrontend/packages/console-shared/src/hooks/__tests__/usePinnedResources.spec.tsfrontend/packages/console-shared/src/hooks/__tests__/useTelemetry.spec.tsfrontend/packages/console-shared/src/hooks/__tests__/useUserPreference.spec.tsfrontend/packages/console-shared/src/hooks/index.tsfrontend/packages/console-shared/src/hooks/useNotificationAlerts.tsfrontend/packages/console-shared/src/hooks/usePinnedResources.tsfrontend/packages/console-shared/src/hooks/useTelemetry.tsfrontend/packages/console-shared/src/hooks/useUserPreference.tsfrontend/packages/console-shared/src/hooks/useUserPreferenceLocalStorage.tsfrontend/packages/console-shared/src/hooks/useUserSettingsCompatibility.tsfrontend/packages/console-telemetry-plugin/src/components/TelemetryUserPreferenceDropdown.tsxfrontend/packages/dev-console/src/components/add/hooks/__tests__/useShowAddCardItemDetails.spec.tsfrontend/packages/dev-console/src/components/add/hooks/useShowAddCardItemDetails.tsfrontend/packages/dev-console/src/components/buildconfig/__tests__/BuildConfigFormPage.spec.tsxfrontend/packages/dev-console/src/components/builds/BuildsTabListPage.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/DeploymentForm.spec.tsxfrontend/packages/dev-console/src/components/deployments/__tests__/EditDeployment.spec.tsxfrontend/packages/dev-console/src/components/import/section/useResourceType.tsfrontend/packages/dev-console/src/components/user-preferences/usePreferredRoutingOptions.tsxfrontend/packages/helm-plugin/src/components/details-page/__tests__/HelmReleaseNotes.spec.tsxfrontend/packages/helm-plugin/src/components/forms/__tests__/HelmInstallUpgradeForm.spec.tsxfrontend/packages/helm-plugin/src/topology/__tests__/TopologyHelmReleaseNotesPanel.spec.tsxfrontend/packages/knative-plugin/src/components/functions/GettingStartedSection.tsxfrontend/packages/operator-lifecycle-manager-v1/src/components/OLMv1Switch.tsxfrontend/packages/operator-lifecycle-manager-v1/src/hooks/useOLMv1FlagProvider.tsfrontend/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-items.tsxfrontend/packages/topology/src/__tests__/TopologyPage.spec.tsxfrontend/packages/topology/src/__tests__/TopologySideBar.spec.tsxfrontend/packages/topology/src/components/dropdowns/NamespaceBarApplicationSelector.tsxfrontend/packages/topology/src/components/export-app/ExportApplicationModal.tsxfrontend/packages/topology/src/components/export-app/__tests__/ExportApplicationModal.spec.tsxfrontend/packages/topology/src/components/export-app/export-app-context.tsfrontend/packages/topology/src/components/graph-view/Topology.tsxfrontend/packages/topology/src/components/page/TopologyPage.tsxfrontend/packages/topology/src/components/side-bar/TopologySideBar.tsxfrontend/packages/topology/src/components/side-bar/components/SideBarAlerts.tsxfrontend/packages/topology/src/filters/FilterProvider.tsxfrontend/packages/topology/src/user-preferences/usePreferredTopologyView.tsfrontend/packages/webterminal-plugin/src/components/cloud-shell/CloudShellTerminal.tsxfrontend/packages/webterminal-plugin/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsxfrontend/public/components/ThemeProvider.tsxfrontend/public/components/__tests__/markdown-view.spec.tsxfrontend/public/components/app-contents.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/getting-started/__tests__/getting-started-section.spec.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/getting-started/getting-started-section.tsxfrontend/public/components/dashboard/dashboards-page/cluster-dashboard/utilization-card.tsxfrontend/public/components/dashboard/project-dashboard/getting-started/GettingStartedSection.tsxfrontend/public/components/dashboard/project-dashboard/getting-started/__tests__/GettingStartedSection.spec.tsxfrontend/public/components/factory/Table/active-columns-hook.tsfrontend/public/components/global-telemetry-notifications.tsxfrontend/public/components/modals/column-management-modal.tsxfrontend/public/components/modals/delete-namespace-modal.tsxfrontend/public/components/modals/edit-yaml-settings-modal.tsxfrontend/public/components/namespace.jsxfrontend/public/components/pod-list.tsxfrontend/public/components/resource-dropdown.tsxfrontend/public/components/useFavoritesOptions.tsfrontend/public/components/utils/console-select.tsxfrontend/public/components/utils/resource-log.tsx
💤 Files with no reviewable changes (6)
- frontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts
- frontend/packages/console-shared/src/hoc/withUserSettings.tsx
- frontend/packages/console-shared/src/hoc/withUserSettingsCompatibility.tsx
- frontend/packages/console-shared/src/components/dropdown/ResourceDropdown.tsx
- frontend/packages/topology/src/components/dropdowns/NamespaceBarApplicationSelector.tsx
- frontend/packages/console-shared/src/hooks/useUserSettingsCompatibility.ts
| const [exactSearch, , exactSearchLoaded] = useUserPreference<boolean>( | ||
| PREFERRED_EXACT_SEARCH_USER_SETTING_KEY, | ||
| ); | ||
| return [exactSearch, exactSearchLoaded]; |
There was a problem hiding this comment.
Return type contract can leak undefined on initial render.
On Line 6, useUserPreference<boolean> is called without a default, but useExactSearch advertises a strict boolean value. Consider defaulting to false to keep the contract truthful.
Proposed fix
export const useExactSearch = (): [boolean, boolean] => {
const [exactSearch, , exactSearchLoaded] = useUserPreference<boolean>(
PREFERRED_EXACT_SEARCH_USER_SETTING_KEY,
+ false,
);
return [exactSearch, exactSearchLoaded];
};📝 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 [exactSearch, , exactSearchLoaded] = useUserPreference<boolean>( | |
| PREFERRED_EXACT_SEARCH_USER_SETTING_KEY, | |
| ); | |
| return [exactSearch, exactSearchLoaded]; | |
| const [exactSearch, , exactSearchLoaded] = useUserPreference<boolean>( | |
| PREFERRED_EXACT_SEARCH_USER_SETTING_KEY, | |
| false, | |
| ); | |
| return [exactSearch, exactSearchLoaded]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-app/src/components/user-preferences/search/useExactSearch.ts`
around lines 6 - 9, The hook useExactSearch currently returns a boolean but
calls useUserPreference<boolean>(PREFERRED_EXACT_SEARCH_USER_SETTING_KEY)
without a default, which can yield undefined on first render; update the call in
useExactSearch to pass a safe default (false) to useUserPreference so
exactSearch is always a boolean, and keep returning [exactSearch,
exactSearchLoaded] from useExactSearch unchanged (useUserPreference must be
invoked with the default false for PREFERRED_EXACT_SEARCH_USER_SETTING_KEY).
| const [preferredCreateEditMethod, , preferredCreateEditMethodLoaded] = useUserPreference<string>( | ||
| PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, examine the file in question
echo "=== File content ==="
cat -n frontend/packages/console-app/src/components/user-preferences/synced-editor/usePreferredCreateEditMethod.ts
echo -e "\n=== Finding useUserPreference definition ==="
# Find where useUserPreference is defined
rg -n "export.*useUserPreference|function useUserPreference|const useUserPreference" --type ts
echo -e "\n=== Check for the constant ==="
# Look for the constant mentioned in the proposed fix
rg -n "PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_VALUE_LATEST" --type ts
echo -e "\n=== Check call sites of usePreferredCreateEditMethod ==="
rg -n "usePreferredCreateEditMethod" --type ts -B2 -A5Repository: openshift/console
Length of output: 18380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== useUserPreference hook definition ==="
cat -n frontend/packages/console-shared/src/hooks/useUserPreference.ts | head -80
echo -e "\n=== Check type definition ==="
rg -n "UseUserPreference|useUserPreference.*=" frontend/packages/console-shared/src/hooks/useUserPreference.ts -A5 | head -40Repository: openshift/console
Length of output: 3996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== useEditorType implementation ==="
cat -n frontend/packages/console-shared/src/components/synced-editor/useEditorType.ts
echo -e "\n=== Check if undefined is guarded in actual usage ==="
rg -n "preferredEditorType" frontend/packages/console-shared/src/components/synced-editor/useEditorType.ts -B2 -A2Repository: openshift/console
Length of output: 3681
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Direct usages of preferredCreateEditMethod in non-test files ==="
rg -n "usePreferredCreateEditMethod" frontend/packages/console-shared/src/components/synced-editor/useEditorType.ts -A20 | grep -E "(preferredEditorType|preferredCreateEditMethod)"
echo -e "\n=== Check for non-guarded access ==="
# Look for access patterns that don't guard
rg -n "preferredEditorType\." frontend/packages/console-shared/src/components/synced-editor/useEditorType.ts
echo -e "\n=== Summary: All call sites for usePreferredCreateEditMethod ==="
rg -l "usePreferredCreateEditMethod" --type ts | grep -v spec | grep -v testRepository: openshift/console
Length of output: 593
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== All non-test files that use usePreferredCreateEditMethod ==="
rg -l "usePreferredCreateEditMethod" --type ts | grep -v spec | grep -v test
echo -e "\n=== Full context for each non-test usage ==="
for file in $(rg -l "usePreferredCreateEditMethod" --type ts | grep -v spec | grep -v test); do
echo "File: $file"
rg -n "usePreferredCreateEditMethod" "$file" -B5 -A10
echo "---"
doneRepository: openshift/console
Length of output: 2492
Add a default value to honor the function's type contract.
The hook declares a return type of [string, boolean] (line 6) but can return [undefined, boolean] when no preference is persisted, since useUserPreference is called without a default value. While the only current call site in useEditorType.ts guards against undefined on line 27, this fragile pattern invites bugs in future consumers who may not add such guards.
Provide the default explicitly:
Proposed fix
export const usePreferredCreateEditMethod = (): [string, boolean] => {
const [preferredCreateEditMethod, , preferredCreateEditMethodLoaded] = useUserPreference<string>(
PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY,
+ PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_VALUE_LATEST,
);
return [preferredCreateEditMethod, preferredCreateEditMethodLoaded];
};📝 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 [preferredCreateEditMethod, , preferredCreateEditMethodLoaded] = useUserPreference<string>( | |
| PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY, | |
| ); | |
| const [preferredCreateEditMethod, , preferredCreateEditMethodLoaded] = useUserPreference<string>( | |
| PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY, | |
| PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_VALUE_LATEST, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-app/src/components/user-preferences/synced-editor/usePreferredCreateEditMethod.ts`
around lines 7 - 9, The hook can return undefined because useUserPreference is
called without a default; update the call to useUserPreference in
usePreferredCreateEditMethod (the line that sets preferredCreateEditMethod using
PREFERRED_CREATE_EDIT_METHOD_USER_SETTING_KEY) to pass an explicit default
string (e.g., '') as the second argument so the hook always returns a string and
satisfies the declared [string, boolean] contract.
| jest.mock('@console/shared/src/hooks/useUserPreference', () => { | ||
| return { | ||
| useUserSettingsCompatibility: () => ['', () => {}], | ||
| useUserPreference: () => ['', () => {}], | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name 'useUserPreference.ts' -o -name 'useUserPreference.tsx' | head -20Repository: openshift/console
Length of output: 126
🏁 Script executed:
find frontend/packages/console-shared/src/components/dropdown -type f -name '*.spec.tsx' | head -10Repository: openshift/console
Length of output: 153
🏁 Script executed:
cat frontend/packages/console-shared/src/components/dropdown/__tests__/ResourceDropdown.spec.tsx | head -30Repository: openshift/console
Length of output: 949
🏁 Script executed:
cat ./frontend/packages/console-shared/src/hooks/useUserPreference.tsRepository: openshift/console
Length of output: 8038
🏁 Script executed:
rg "useUserPreference" frontend/packages/console-shared/src/components/dropdown/ResourceDropdown.tsx -C3Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat frontend/packages/console-shared/src/components/dropdown/ResourceDropdown.tsx | head -80Repository: openshift/console
Length of output: 3062
🏁 Script executed:
rg "useUserPreference" frontend/packages/console-shared/src/components/dropdown/ -C3Repository: openshift/console
Length of output: 1153
🏁 Script executed:
rg "useUserPreference" frontend/packages/console-shared/src/components/dropdown/ --type tsx --type tsRepository: openshift/console
Length of output: 88
🏁 Script executed:
grep -r "useUserPreference" frontend/packages/console-shared/src/components/utils/console-select.tsx 2>/dev/null | head -20Repository: openshift/console
Length of output: 43
🏁 Script executed:
find frontend/packages/console-shared -name 'ConsoleSelect.tsx' -o -name 'console-select.tsx'Repository: openshift/console
Length of output: 43
🏁 Script executed:
rg "useUserPreference" frontend/packages/console-shared/src/components/ -t ts -t jsx -t tsxRepository: openshift/console
Length of output: 88
🏁 Script executed:
find frontend -name 'ConsoleSelect*' | head -5Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat frontend/packages/internal/src/components/utils/console-select.tsx | head -100Repository: openshift/console
Length of output: 159
🏁 Script executed:
rg "useUserPreference" frontend/packages/console-shared/src/components/Repository: openshift/console
Length of output: 2605
🏁 Script executed:
find frontend/packages -name '*console-select*'Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat frontend/packages/internal/src/components/utils/console-select/ConsoleSelect.tsx | head -150Repository: openshift/console
Length of output: 173
🏁 Script executed:
rg "useUserPreference" frontend/packages/internal/src/components/utils/console-select/ -C 2Repository: openshift/console
Length of output: 164
🏁 Script executed:
cat frontend/packages/internal/src/components/utils/console-select/ConsoleSelect.tsx | grep -A 5 "useUserPreference"Repository: openshift/console
Length of output: 173
🏁 Script executed:
grep -r "useUserPreference" frontend/packages/internal/src/components/utils/console-select/ 2>/dev/nullRepository: openshift/console
Length of output: 43
🏁 Script executed:
find frontend -name 'ConsoleSelect*' -type fRepository: openshift/console
Length of output: 43
🏁 Script executed:
find frontend -name '*console-select*' -type fRepository: openshift/console
Length of output: 112
🏁 Script executed:
rg "class ConsoleSelect|export.*ConsoleSelect" frontend/ --max-count=5Repository: openshift/console
Length of output: 543
🏁 Script executed:
cat frontend/public/components/utils/console-select.tsx | grep -A 50 "export const ConsoleSelect"Repository: openshift/console
Length of output: 1478
🏁 Script executed:
rg "useUserPreference" frontend/public/components/utils/console-select.tsxRepository: openshift/console
Length of output: 196
🏁 Script executed:
cat frontend/packages/console-shared/src/components/dropdown/ResourceDropdown.tsx | grep -A 20 "class ResourceDropdownInternal"Repository: openshift/console
Length of output: 767
Update mock tuple to match useUserPreference contract.
The hook returns [value, setter, loaded], but this mock returns only [value, setter]. The loaded flag is part of the real API signature. While the component may not destructure it in this particular usage, the mock should faithfully represent the actual hook contract to prevent test drift and potential loading-state bugs.
Proposed fix
jest.mock('@console/shared/src/hooks/useUserPreference', () => {
return {
- useUserPreference: () => ['', () => {}],
+ useUserPreference: () => ['', () => {}, true],
};
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-shared/src/components/dropdown/__tests__/ResourceDropdown.spec.tsx`
around lines 6 - 9, The mock for useUserPreference should match the real hook
signature [value, setter, loaded]; update the jest.mock in
ResourceDropdown.spec.tsx so the mocked useUserPreference returns a 3-tuple
(e.g. ['', () => {}, true]) instead of two items, ensuring the loaded flag is
present; locate the mock definition for useUserPreference and adjust its return
to include the third boolean element.
| const favoritesUserPreferenceKey = `${NAMESPACE_USERSETTINGS_PREFIX}.bookmarks`; | ||
| const systemNamespacesSettingsKey = `${NAMESPACE_USERSETTINGS_PREFIX}.systemNamespace`; | ||
| const favoriteStorageKey = `${NAMESPACE_LOCAL_STORAGE_KEY}-bookmarks`; | ||
| const systemNamespaceKey = `${NAMESPACE_LOCAL_STORAGE_KEY}-systemNamespace`; | ||
| const [favorites, setFavorites] = useUserSettingsCompatibility( | ||
| favoritesUserSettingsKey, | ||
| favoriteStorageKey, | ||
| const [favorites, setFavorites] = useUserPreference( | ||
| favoritesUserPreferenceKey, | ||
| undefined, | ||
| true, | ||
| ); |
There was a problem hiding this comment.
Dormant-user legacy preferences can be dropped with this direct hook switch.
Line 232 and Line 280 now rely on useUserPreference directly, so the old one-time localStorage migration path is no longer exercised in this component. Users returning after a long gap may lose legacy bookmarks/system-namespace settings.
Please either add a one-time fallback for these two legacy values or explicitly track/accept this upgrade behavior.
Also applies to: 280-284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/console-shared/src/components/namespace/NamespaceDropdown.tsx`
around lines 230 - 236, The current replacement of legacy migration with direct
useUserPreference (keys favoritesUserPreferenceKey and
systemNamespacesSettingsKey) can drop dormant-user settings; add a one-time
migration fallback: when useUserPreference returns undefined for favorites or
system namespace, read the legacy localStorage keys (the old
bookmarks/system-namespace keys), validate/parse them, then call the
corresponding setter (setFavorites or the system namespace setter used near
lines referencing systemNamespacesSettingsKey) to persist the migrated value and
optionally mark a migrated flag so this runs only once; alternatively,
explicitly document/emit a telemetry/upgrade flag if you intend to accept loss
of legacy data.
| export type WithUserPreferenceProps<T> = { | ||
| userSettingState: T; | ||
| setUserSettingState: (v: T) => void; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Examine the withUserPreference HOC file around lines 4-7 and 17-20
cat -n frontend/packages/console-shared/src/hoc/withUserPreference.tsx | head -25Repository: openshift/console
Length of output: 1113
🏁 Script executed:
#!/bin/bash
# Find and examine the useUserPreference hook
find frontend -name '*useUserPreference*' -type f | head -10Repository: openshift/console
Length of output: 279
🏁 Script executed:
#!/bin/bash
# Check if column-management-modal uses updater functions
rg -A 5 -B 5 'setTableColumns' frontend/public/components/modals/column-management-modal.tsx | head -40Repository: openshift/console
Length of output: 978
🏁 Script executed:
#!/bin/bash
cat -n frontend/packages/console-shared/src/hooks/useUserPreference.tsRepository: openshift/console
Length of output: 9662
Use React state-dispatch typing for setUserSettingState.
The HOC currently narrows the setter to (v: T) => void, but useUserPreference actually returns Dispatch<SetStateAction<T>> (line 194 of the hook). The hook explicitly handles functional updaters (line 197–198), yet consumers like ColumnManagementModal (which uses setTableColumns((prevState) => {...})) are forced to work around the narrowed type contract. Fix the HOC's public type to preserve the full React setter semantics.
🔧 Proposed fix
-import type { ComponentType, FC } from 'react';
+import type { ComponentType, Dispatch, FC, SetStateAction } from 'react';
import { useUserPreference } from '../hooks';
export type WithUserPreferenceProps<T> = {
userSettingState: T;
- setUserSettingState: (v: T) => void;
+ setUserSettingState: Dispatch<SetStateAction<T>>;
};
@@
- const Component = (props: Props) => {
+ const Component = (props: Omit<Props, keyof WithUserPreferenceProps<T>>) => {
const [state, setState, loaded] = useUserPreference(key, defaultValue, sync);
return loaded ? (
<WrappedComponent {...props} userSettingState={state} setUserSettingState={setState} />
) : null;
};Also applies to: 17–20
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/console-shared/src/hoc/withUserPreference.tsx` around lines
4 - 7, Update the WithUserPreferenceProps generic to type the setter as React's
Dispatch<SetStateAction<T>> instead of (v: T) => void so consumers can pass
functional updaters; specifically change the setUserSettingState declaration in
WithUserPreferenceProps to use Dispatch<SetStateAction<T>> (matching
useUserPreference's return type and internal handling) so components like
ColumnManagementModal can call setTableColumns(prev => ...) without type errors.
| jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ | ||
| useUserPreference: () => ['', () => {}], | ||
| })); |
There was a problem hiding this comment.
Mock the full useUserPreference tuple to avoid test-path drift.
The mock currently returns only [value, setter]. useUserPreference returns [value, setter, loaded], so loaded becomes undefined and can alter render branches in tests.
✅ Proposed fix
jest.mock('@console/shared/src/hooks/useUserPreference', () => ({
- useUserPreference: () => ['', () => {}],
+ useUserPreference: () => ['', () => {}, true],
}));📝 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.
| jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ | |
| useUserPreference: () => ['', () => {}], | |
| })); | |
| jest.mock('@console/shared/src/hooks/useUserPreference', () => ({ | |
| useUserPreference: () => ['', () => {}, true], | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/packages/helm-plugin/src/components/forms/__tests__/HelmInstallUpgradeForm.spec.tsx`
around lines 54 - 56, The mock for useUserPreference in the test returns only a
two-element tuple which leaves the third `loaded` value undefined and can change
render branches; update the jest.mock for
'@console/shared/src/hooks/useUserPreference' so useUserPreference returns the
full three-element tuple (e.g. [value, setter, loaded])—set the third element to
an explicit boolean (true or false) appropriate for the test scenario—to ensure
component rendering paths are deterministic.
| it('should allow setting default to list view', () => { | ||
| (useUserSettingsCompatibility as jest.Mock).mockReturnValue(['', () => {}]); | ||
| (useUserPreference as jest.Mock).mockReturnValue(['', () => {}]); | ||
| (usePreferredTopologyView as jest.Mock).mockReturnValue(['', true]); | ||
| (useQueryParams as jest.Mock).mockReturnValue(new URLSearchParams('view=list')); | ||
| renderWithProviders(<TopologyPage defaultViewType={TopologyViewType.list} />); | ||
| expect(screen.getByTestId('topology-list-page')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
Inconsistent mock return values missing loaded boolean.
Lines 98 and 152 return only two elements ['', () => {}] while other test cases correctly return the full tuple including the loaded boolean. This inconsistency could mask bugs in components that rely on the loaded state.
🐛 Proposed fix for consistent mock returns
it('should allow setting default to list view', () => {
- (useUserPreference as jest.Mock).mockReturnValue(['', () => {}]);
+ (useUserPreference as jest.Mock).mockReturnValue(['', () => {}, true]);
(usePreferredTopologyView as jest.Mock).mockReturnValue(['', true]); it('should support URL view=list', () => {
- (useUserPreference as jest.Mock).mockReturnValue(['', () => {}]);
+ (useUserPreference as jest.Mock).mockReturnValue(['', () => {}, true]);
(usePreferredTopologyView as jest.Mock).mockReturnValue(['', true]);Also applies to: 151-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/topology/src/__tests__/TopologyPage.spec.tsx` around lines
97 - 103, Mocks for useUserPreference and usePreferredTopologyView in
TopologyPage.spec.tsx are returning only two items and must include the third
`loaded` boolean; update the mocked returns (e.g., change (useUserPreference as
jest.Mock).mockReturnValue(['', () => {}]) to mockReturnValue(['', () => {},
true]) and change (usePreferredTopologyView as jest.Mock).mockReturnValue(['',
true]) to mockReturnValue(['', true, true]) (and apply the same 3-element tuple
fix for the other test block mentioned) so the component sees the full [value,
setter, loaded] tuple during tests.
| const [appliedFilters, setAppliedFilters, appliedFiltersLoaded] = useUserPreference( | ||
| TOPOLOGY_DISPLAY_FILTERS_USER_SETTINGS_KEY, | ||
| TOPOLOGY_DISPLAY_FILTERS_LOCAL_STORAGE_KEY, | ||
| getAppliedFilters(DEFAULT_TOPOLOGY_FILTERS), | ||
| ); |
There was a problem hiding this comment.
Potential legacy preference loss after removing compatibility migration.
At Line 40, replacing the compatibility hook with useUserPreference drops the implicit one-time localStorage migration path. Users who never logged in during the migration window can silently lose saved topology filters after upgrade. Please add a one-time fallback migration in this flow (or an explicit, documented product decision with guardrails).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/topology/src/filters/FilterProvider.tsx` around lines 40 -
43, The current useUserPreference call for
TOPOLOGY_DISPLAY_FILTERS_USER_SETTINGS_KEY loses any legacy localStorage-stored
filters; add a one-time fallback migration: keep the
useUserPreference([appliedFilters, setAppliedFilters, appliedFiltersLoaded])
call, then in a useEffect that runs when appliedFiltersLoaded becomes true and
appliedFilters equals getAppliedFilters(DEFAULT_TOPOLOGY_FILTERS), read the
legacy storage key (e.g., LEGACY_TOPOLOGY_FILTERS_KEY) from localStorage, parse
and validate it via getAppliedFilters, call setAppliedFilters(migratedValue) if
valid, and then remove the legacy key so the migration runs only once; reference
appliedFilters, setAppliedFilters, appliedFiltersLoaded, useUserPreference,
TOPOLOGY_DISPLAY_FILTERS_USER_SETTINGS_KEY, getAppliedFilters, and
DEFAULT_TOPOLOGY_FILTERS in the change.
| const [isGettingStartedSectionOpen, setIsGettingStartedSectionOpen] = useUserPreference<boolean>( | ||
| `${userSettingKey}.expanded`, | ||
| true, | ||
| ); |
There was a problem hiding this comment.
Guard against transient undefined from useUserPreference.
useUserPreference may yield undefined before initial load, but isOpen expects a boolean. Consider a safe fallback (?? true) or gate on the loaded flag to avoid first-render state glitches.
Suggested fix
- const [isGettingStartedSectionOpen, setIsGettingStartedSectionOpen] = useUserPreference<boolean>(
+ const [isGettingStartedSectionOpen, setIsGettingStartedSectionOpen] = useUserPreference<boolean>(
`${userSettingKey}.expanded`,
true,
);
@@
- isOpen={isGettingStartedSectionOpen}
+ isOpen={isGettingStartedSectionOpen ?? true}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/public/components/dashboard/project-dashboard/getting-started/GettingStartedSection.tsx`
around lines 24 - 27, The call to useUserPreference can briefly return undefined
while loading, causing isGettingStartedSectionOpen (from useUserPreference) to
be undefined when consumers expect a boolean; update the initialization in
GettingStartedSection.tsx to guard against transient undefined by using a safe
fallback (e.g., coalesce isGettingStartedSectionOpen with true via `?? true`) or
gate rendering/behavior on the preference's loaded flag returned by
useUserPreference; update references to isGettingStartedSectionOpen and
setIsGettingStartedSectionOpen accordingly so consumers always see a boolean.
| const bookmarkUserPreferenceKey = userSettingsPrefix | ||
| ? `${userSettingsPrefix}.bookmarks` | ||
| : undefined; | ||
| const bookmarkStorageKey = storageKey ? `${storageKey}-bookmarks` : undefined; | ||
|
|
||
| const enableBookmarks = !!bookmarkUserSettingsKey || !!bookmarkStorageKey; | ||
| const enableBookmarks = !!bookmarkUserPreferenceKey; | ||
|
|
||
| const [bookmarks, setBookmarks] = useUserSettingsCompatibility( | ||
| bookmarkUserSettingsKey, | ||
| bookmarkStorageKey, | ||
| const [bookmarks, setBookmarks] = useUserPreference( | ||
| bookmarkUserPreferenceKey, | ||
| {}, | ||
| true, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check useUserPreference implementation for undefined key handling
echo "=== Check useUserPreference implementation for undefined key handling ==="
rg -n -A 30 "export const useUserPreference" --type ts --type tsx | head -100Repository: openshift/console
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Check the file structure to locate the hook properly
echo "=== Find useUserPreference hook ==="
fd -e ts -e tsx | xargs rg -l "useUserPreference" | grep -i hook | head -10Repository: openshift/console
Length of output: 794
🏁 Script executed:
#!/bin/bash
# Look for the hook in console-shared package
echo "=== Search in console-shared for useUserPreference ==="
find . -path "*/console-shared/*" -name "*.ts" -o -name "*.tsx" | xargs rg -n "export.*useUserPreference" 2>/dev/null | head -20Repository: openshift/console
Length of output: 638
🏁 Script executed:
#!/bin/bash
# Read the useUserPreference hook implementation
echo "=== useUserPreference hook implementation ==="
cat -n frontend/packages/console-shared/src/hooks/useUserPreference.ts | head -100Repository: openshift/console
Length of output: 4136
🏁 Script executed:
#!/bin/bash
# Read the test file to understand expected behavior
echo "=== useUserPreference test file ==="
cat -n frontend/packages/console-shared/src/hooks/__tests__/useUserPreference.spec.tsRepository: openshift/console
Length of output: 32388
🏁 Script executed:
#!/bin/bash
# Read more of the hook to see how keyRef is used
cat -n frontend/packages/console-shared/src/hooks/useUserPreference.ts | sed -n '100,250p'Repository: openshift/console
Length of output: 5644
🏁 Script executed:
#!/bin/bash
# Check specifically for keyRef usage
rg -n "keyRef" frontend/packages/console-shared/src/hooks/useUserPreference.tsRepository: openshift/console
Length of output: 812
🏁 Script executed:
#!/bin/bash
# Check if there's any test case that passes undefined as key
rg -n "undefined" frontend/packages/console-shared/src/hooks/__tests__/useUserPreference.spec.ts | head -20Repository: openshift/console
Length of output: 879
Conditionally invoke useUserPreference or handle undefined keys in the hook.
When userSettingsPrefix is not provided, bookmarkUserPreferenceKey will be undefined, and useUserPreference is called with an undefined key. The hook implementation uses optional chaining on the cfData object but not on keyRef.current itself. This results in operations like cfData.data[keyRef.current] attempting to access ConfigMap data with an undefined key, and updateConfigMap being invoked with undefined as the key parameter—both incorrect behaviors. While the enableBookmarks check guards downstream usage, the hook is invoked unconditionally, wasting resources and potentially creating invalid ConfigMap or localStorage entries.
Either conditionally call the hook only when userSettingsPrefix is defined, or modify the hook to return no-op values (empty state and setter) when the key parameter is undefined.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/utils/console-select.tsx` around lines 147 - 157,
The code calls useUserPreference with bookmarkUserPreferenceKey which can be
undefined; either stop calling useUserPreference when bookmarkUserPreferenceKey
is undefined (wrap the useUserPreference call behind the existing
enableBookmarks check so bookmarks and setBookmarks are only created when
enableBookmarks is true), or update the useUserPreference implementation to
early-return a no-op pair (empty state and a setter that does nothing) whenever
its key parameter is undefined (ensure keyRef.current and any cfData access and
updateConfigMap calls are guarded against undefined). Reference:
bookmarkUserPreferenceKey, enableBookmarks, useUserPreference, keyRef.current,
cfData, updateConfigMap.
|
@vojtechszocs: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
Depends on #16057
useUserPreferenceCompatibilityandwithUserPreferenceCompatibilityuseUserPreferenceCompatibilitywithuseUserPreferencewithUserPreferenceCompatibilitywithwithUserPreferencewithUserPreference- addedsyncparameterClaude Code - Analysis on code removal
The hook at packages/console-shared/src/hooks/useUserPreferenceCompatibility.ts:7-32 is a thin wrapper around useUserPreference that does exactly two extra things:
If it does, it deserializes that value and passes it as the defaultValue to useUserPreference (instead of the caller's defaultValue).
This means old localStorage data gets picked up and written into the ConfigMap on first use.
successfully from the ConfigMap, it removes the old storageKey from localStorage.
In essence, it's a one-time migration bridge: it moves data from a legacy per-key localStorage entry into the ConfigMap-backed useUserPreference system, then deletes the old localStorage entry.
Yes, useUserPreferenceCompatibility can be safely removed in favor of useUserPreference. The compatibility hook's only purpose — migrating data from legacy per-feature localStorage keys to ConfigMap storage — is a one-time operation that has already occurred for active users. After migration, the compatibility hook is functionally identical to useUserPreference. The withUserPreferenceCompatibility HOC (which wraps the hook) can similarly be replaced with a withUserPreference HOC or refactored to use the hook directly.
The one edge case to consider: a user who hasn't logged in since the migration was introduced would lose their old localStorage preferences. Given the time that has likely elapsed, this is an acceptable trade-off, and the data loss would be limited to UI preferences (column ordering, pinned resources, etc.) that are easily re-set.
Summary by CodeRabbit
Release Notes