CONSOLE-5300: Parallelize context detection to reduce initial load time#16477
Conversation
|
@logonoff: This pull request references CONSOLE-5300 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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. |
📝 WalkthroughWalkthroughThis PR consolidates three separate asynchronous detection components—perspective, namespace, and language—into a single 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/packages/console-app/src/components/detect-context/DetectContext.tsx`:
- Around line 108-109: The readiness gate currently ignores the
preferred-language loading state so the app can render before the locale is
applied; update the readiness logic to include preferredLanguageLoaded and defer
calling useLanguage until that flag is true. Specifically, incorporate
preferredLanguageLoaded into the ready/pending evaluation used in DetectContext
(where usePreferredLanguage() is called) and wrap the
useLanguage(preferredLanguage, preferredLanguageLoaded) invocation so it only
runs when preferredLanguageLoaded is true (and ensure any effects depending on
ready/pending also account for preferredLanguageLoaded). Apply the same change
to the other occurrence in this file that mirrors lines 133–146 so both language
hooks participate in the startup readiness gate.
- Around line 128-130: The call to applyReduxExtensions (which calls
store.replaceReducer) must not run during render; move the conditional block
that currently runs when reducersResolved into a React effect by creating a
useEffect hook that watches [reducersResolved, reduxReducerExtensions] and
invokes applyReduxExtensions(reduxReducerExtensions) only when reducersResolved
is true; ensure you reference the existing symbols (reducersResolved,
reduxReducerExtensions, applyReduxExtensions) and remove the direct call from
render so reducer replacement only happens inside the effect.
- Around line 82-89: The provider nesting is reversed because
contextProviderExtensions is reduced left-to-right; change the call to use
reduceRight so the original extension order is preserved (first extension
becomes the outermost). Specifically, replace the use of
contextProviderExtensions.reduce(...) that wraps children with <EnhancedProvider
key={e.uid} {...e.properties}> with contextProviderExtensions.reduceRight(...),
keeping the same accumulator structure and children variable, so
EnhancedProvider and keys remain unchanged.
In `@frontend/public/style/_layout.scss`:
- Line 1: After switching to "`@use` './vars';" the Sass variables $screen-md-min
and $screen-sm-max are no longer global, so update all references in
_layout.scss to use the module namespace (vars.$screen-md-min and
vars.$screen-sm-max) wherever those unprefixed breakpoint variables are used
(eg. in media queries or mixin calls) so compilation succeeds with the vars
module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 414a7fe7-b6fe-42ca-b20d-075857b60acd
📒 Files selected for processing (23)
frontend/packages/console-app/package.jsonfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/PerspectiveConfiguration.tsxfrontend/packages/console-app/src/components/detect-context/PerspectiveDetector.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/PerspectiveDetector.spec.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/checkNamespaceExists.spec.tsfrontend/packages/console-app/src/components/detect-context/__tests__/getValueForNamespace.spec.tsfrontend/packages/console-app/src/components/detect-context/__tests__/namespace.spec.tsfrontend/packages/console-app/src/components/detect-context/__tests__/useValuesForPerspectiveContext.spec.tsfrontend/packages/console-app/src/components/detect-context/checkNamespaceExists.tsfrontend/packages/console-app/src/components/detect-context/getValueForNamespace.tsfrontend/packages/console-app/src/components/detect-context/namespace.tsfrontend/packages/console-app/src/components/detect-context/useLastNamespace.tsfrontend/packages/console-app/src/components/detect-context/useLastPerspective.tsfrontend/packages/console-app/src/components/detect-context/useValuesForPerspectiveContext.tsfrontend/packages/console-app/src/components/detect-language/DetectLanguage.tsxfrontend/packages/console-app/src/components/detect-namespace/DetectNamespace.tsxfrontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsxfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/public/components/app.tsxfrontend/public/style/_layout.scss
💤 Files with no reviewable changes (3)
- frontend/packages/console-app/src/components/detect-namespace/DetectNamespace.tsx
- frontend/packages/console-app/src/components/detect-language/DetectLanguage.tsx
- frontend/packages/console-app/src/components/detect-perspective/DetectPerspective.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/**/*.{ts,tsx,js,jsx}: Never import from package index files (e.g.,@console/shared) in new code, as they can create circular dependencies and slow builds. Import from specific file paths instead.
Do not use backticks int()calls for i18n strings, as the i18n parser cannot extract keys from template literals. Use single or double quotes instead.
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import from deprecated packages or use code with the
@deprecatedTSdoc tag in new code.
**/*.{ts,tsx}: Use React functional components with hooks instead of class components
State Management: Use React hooks and Context API (migrating away from legacy Redux/Immutable.js)
Hooks: Use existing hooks fromconsole-sharedwhen possible (useK8sWatchResource,useUserSettings, etc.)
API calls: Use k8s resource hooks for data fetching,consoleFetchJSONfor HTTP requests
Extensions: Use console extension points for plugin integration
Types: Check existing types inconsole-sharedbefore creating new ones
Dynamic Plugins: Use console extension points for plugin integration
Styling: Use SCSS modules co-located with components, PatternFly design system components, avoid any SCSS/CSS if possible
Accessibility: Follow WCAG 2.1 AA standards, use semantic HTML, ARIA labels where needed, ensure keyboard navigation, test with screen readers
i18n: UseuseTranslation('namespace')hook withkeyformat for translation keys
Error Handling: Use ErrorBoundary components and graceful degradation patterns
Optimize re-renders: UseuseCallbackfor memoized callbacks to avoid function recreation every render
Optimize re-renders: UseuseMemofor expensive computations to avoid recalculating on every render
Lazy loading: UseReact.lazy()to lazy load heavy components
TypeScript type safety: Avoid usinganytype; suggest proper type definitions and verify null/undefined are handled properly
Type component props properly: Reuse existing component prop types instead of duplicating type definitions
Use proper hooks: Use specialized hooks likeusePluginInfofor plugin data instead of generic data fetching patterns
Avoid deprecated components: Check for JSDoc@deprecatedtags, import paths containing/deprecated, andDEPRECATED_file name prefix before using components
Importing from barrel files and circular dependencies: Import directly from specific files instead...
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
frontend/**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Never use absolute URLs or paths in the console code. The console runs behind a proxy under an arbitrary path.
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/package.jsonfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When writing code for static plugins, ensure that all
$codeRefreference the corresponding extension type from the@console/dynamic-plugin-sdkpackage.
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (TESTING.md)
**/*.{tsx,ts}: Always usepage.getByTestId('x')for Playwright selectors which queries[data-test="x"]. If a React element only has a legacy test attribute, adddata-testto the element. Never remove legacy attributes
Preferdata-testattributes in Cypress selectors (e.g.,cy.get('[data-test="create-deployment"]')) over brittle CSS/ARIA selectorsFile Naming: PascalCase for components, kebab-case for utilities,
*.spec.ts(x)for tests
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.{go,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Use lowercase dash-separated names for all files to avoid git issues with case-insensitive file systems
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
**/*.{ts,tsx,js,jsx}: New code MUST be written in TypeScript, not JavaScript
Prefer functional programming patterns and immutable data structures
Run the linter and follow all rules defined in .eslintrc
Never use absolute paths in code - the app should be able to run behind a proxy under an arbitrary path
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.ts
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Plugin SDK Changes: Any updates to
console-dynamic-plugin-sdkshould aim to maintain backward compatibility as it's a public API - use theplugin-api-reviewskill to vet changes for public API impact and ensure proper documentation updates
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.ts
frontend/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (README.md)
frontend/**/*.{js,ts,tsx}: Support only the latest versions of Edge, Chrome, Safari, and Firefox browsers; IE 11 and earlier are not supported
CSP violations should be automatically reported to telemetry by parsing dynamic plugin names from securitypolicyviolation events, with throttling to prevent duplicate reports within a day
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (INTERNATIONALIZATION.md)
For dynamic translation keys that cannot be parsed by i18next-parser (t(key), t('key' + id), t(
key${id})), specify possible static values in comments for the parser to extract
Files:
frontend/packages/console-shared/src/hooks/useActiveNamespace.tsfrontend/packages/console-dynamic-plugin-sdk/src/api/internal-api.tsfrontend/packages/console-app/src/components/detect-context/DetectContext.tsxfrontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsxfrontend/public/components/app.tsx
**/__tests__/**/*.spec.tsx
📄 CodeRabbit inference engine (TESTING.md)
**/__tests__/**/*.spec.tsx: Unit tests must use Jest framework with@testing-library/reactand@testing-library/jest-domlibraries for testing React components, hooks, and utilities
Test what users see and interact with - DO NOT test internal component state, private methods, props passed to child components, CSS class names/styles, or component structure
Use accessibility-first queries that match how screen readers and users interact with the UI
Always prefer role-based queries (e.g.,getByRole) over generic selectors for semantic testing
Use reusable helper functions such asrenderWithProvidersandrenderHookWithProvidersfromfrontend/packages/console-shared/src/test-utils. Extract repetitive setup not covered by these helpers into custom functions if needed
Handle asynchronous updates withfindBy*andwaitForin tests
Use proper TypeScript types for props, state, and mock data in unit tests
Structure tests following the Arrange-Act-Assert (AAA) pattern: Arrange (render component with mocks), Act (perform user actions), Assert (verify expected state)
Test files must be located in__tests__/directory within the component directory, use the same name as the implementation file, and use.spec.tsxextension
When mocking, ALWAYS use ESMimportstatements at the top of the file - NEVER userequire('react')orReact.createElement()in mocks
Preferjest.mock()for module mocks andjest.fn()for component mocks instead ofjest.spyOn()
Keep mocks simple - returnnull, strings, orchildrendirectly. Usejest.fn(() => null)for simple component mocks,jest.fn(() => 'ComponentName')for mocks that display text, andjest.fn((props) => props.children)for wrapper components
Mock custom hooks withjest.fn()returning mock data
Clean up mocks withafterEach(() => { jest.restoreAllMocks(); })
Files:
frontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (STYLEGUIDE.md)
Tests should follow a similar 'test tables' convention as used in Go where applicable
Files:
frontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsx
frontend/**/*.{scss,css}
📄 CodeRabbit inference engine (AGENTS.md)
Exhaust PatternFly component options before writing custom SCSS. When custom SCSS is necessary, use BEM naming convention with
co-prefix.
Files:
frontend/public/style/_layout.scss
**/*.scss
📄 CodeRabbit inference engine (STYLEGUIDE.md)
**/*.scss: Use lowercase dash-separated names for all files to avoid git issues with case-insensitive file systems
When possible, avoid writing any custom SCSS/CSS and use PatternFly for all styling
Avoid element selectors when possible; class selectors are preferred
Scope all classes with a recognizable prefix to avoid collisions with imported CSS (this project usesco-by convention)
Class names should be all lowercase and dash-separated
All SCSS variables should be scoped within their component
Use BEM (Block Element Modifier) naming conventions
Files:
frontend/public/style/_layout.scss
**/_*.scss
📄 CodeRabbit inference engine (STYLEGUIDE.md)
All SCSS files should be prefixed with an underscore (eg
_my-custom-file.scss)
Files:
frontend/public/style/_layout.scss
15d25b5 to
28d4ec6
Compare
|
/label px-approved no api changes |
|
/test all |
| * Runs perspective, namespace, language, and extension resolution hooks at the | ||
| * same level so their async work executes in parallel, then provides the | ||
| * resolved values via PerspectiveContext, NamespaceContext, and | ||
| * ContextProviderExtensionsContext. |
There was a problem hiding this comment.
I think it would be helpful to provide an explicit bullet list of all kinds of detections performed by this component.
- detect active perspective
- detect active namespace
- detect preferred language
- resolve all
ReduxReducerandContextProviderextensions
28d4ec6 to
aad0384
Compare
Consolidate DetectPerspective, DetectNamespace, DetectLanguage, and AppWithExtensions into a single DetectContext component. Previously these ran sequentially — namespace detection could not start until perspective detection completed, and neither could start until extension resolution finished. Now all hooks fire at the same React component level, reducing load time from the sum of all operations to the duration of the longest one. - Merge detect-perspective/, detect-namespace/, and detect-language/ into a single detect-context/ directory - Replace bare LoadingBox spinners with a PageSkeleton that renders an empty PatternFly Page layout during initialization - Dynamically track pending subsystems in the skeleton blame label - Preserve plugin context provider nesting order to avoid breaking dynamic plugins Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
aad0384 to
f7aa632
Compare
|
/lgtm |
QA Verification Evidence
Verification Steps
Step 5: Deployments list (openshift-console) (pass)
Step 8: Time-to-Content BenchmarkMeasures time from navigation start ( Raw measurements (click to expand)
Note The candidate shows a ~10% improvement in time-to-content across 10 runs (average and median). Both branches show high variance (~500-600ms stdev) typical of a local dev environment, but the improvement is consistent across all summary metrics. The PR's parallelization benefit would be more pronounced on production clusters where API latencies are higher and the sequential-to-parallel conversion eliminates compounding network round-trips. Console Errors
No new console errors introduced. Warning This verification was performed by an AI agent. Results may contain false positives or miss Automated QA verification by Claude Code |
Fire a lightweight coFetch to /api/kubernetes/api at module load time, before graphQLReady or DetectContext mount. If the user is not authenticated, coFetch's 401 interceptor triggers authSvc.handle401() and redirects to the login page immediately, preventing a flash of the PageSkeleton for unauthenticated users. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2deb594 to
66779e7
Compare
|
/verified by claude |
|
@logonoff: This PR has been marked as verified by 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. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, 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 |
|
@logonoff: all tests passed! 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. |
















Analysis / Root cause:
The console's initial load path runs context detection sequentially:
AppWithExtensionsblocks until plugin extensions resolve, thenDetectPerspectiveblocks until perspective user preferences load, thenDetectNamespaceblocks until namespace resolution completes (including K8s API calls). Each stage must fully complete before the next can begin, making the total startup time the sum of all operations rather than the maximum.Solution description:
Consolidate
DetectPerspective,DetectNamespace,DetectLanguage, andAppWithExtensionsinto a singleDetectContextcomponent that calls all initialization hooks at the same React component level. This means all async work (user preference loads, K8s API calls, extension resolution) fires in parallel on mount, reducing startup time fromT_extensions + T_perspective + T_namespacetomax(T_all).Key changes:
detect-context/directory: Mergesdetect-perspective/,detect-namespace/, anddetect-language/into one locationDetectContextcomponent: Runs all detection and extension resolution hooks at the same level, providesPerspectiveContext,NamespaceContext, and resolved plugin context provider extensionsPageSkeletoncomponent: Replaces bareLoadingBoxspinners with an empty PatternFlyPagelayout (masthead + sidebar + content area) during initialization, preventing layout shift"Perspective, Namespace, Reducers")ContextProviderExtensionWrapper: Reads resolved plugin context providers from an internal context and wraps children, preserving the original nesting order (insideModalProvider/OverlayProvider) to avoid breaking dynamic pluginsAppWithExtensionsremoved: Extension resolution moved intoDetectContext,Apprenders directly fromAppRouterapp.tsxsimplified: RemovedEnhancedProvider, extension-related imports, and thecontextProviderExtensionsprop fromAppScreenshots / screen recording:
After is first
Screen.Recording.2026-05-20.at.5.13.16.PM.mov
Test setup:
No special setup required. The
?crp-blamequery parameter can be used to see which subsystems are loading in the skeleton.Test cases:
?perspective=devparameter)Browser conformance:
Additional info:
The plugin context provider nesting order is preserved — plugin-provided contexts remain inside
ModalProvider>OverlayProvider, matching the original hierarchy. This ensures no breaking changes for dynamic plugins that depend on modal or overlay APIs from within their context providers.Reviewers and assignees:
Summary by CodeRabbit
Release Notes
New Features
Refactor