use user Profile instead of user#4005
Conversation
📝 WalkthroughWalkthroughThe PR migrates the React entrypoint from ChangesSilent Renew Auth Flow and Entrypoint Migration
Redux userProfile Selector Refinement
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…into cleanup-apache-config-requests
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/components/silent-renew-app.tsx (1)
30-34: 💤 Low valueSimplify the callback - the null check is redundant.
Since
handleSilentRenewCallbackClosureis recreated wheneveruserManagerchanges (it's in the dependency array), the null check on line 31 will never be true when the callback is invoked afteruserManageris set. You can either remove the check or passuserManagerdirectly to the handler.♻️ Proposed simplification
Option 1: Remove the null check
const handleSilentRenewCallbackClosure = useCallback(() => { - if (userManager) { - handleSilentRenewCallback(userManager); - } + handleSilentRenewCallback(userManager!); }, [userManager]);Option 2: Keep it defensive and clear
The current implementation is defensive and readable. If you prefer safety over minor optimization, the existing code is fine.
🤖 Prompt for 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. In `@src/components/silent-renew-app.tsx` around lines 30 - 34, The null check on userManager inside the handleSilentRenewCallbackClosure callback is redundant because userManager is included in the dependency array, ensuring the callback is only recreated when userManager changes and will always be defined when the callback executes. Remove the if (userManager) condition and directly call handleSilentRenewCallback(userManager) to simplify the code while maintaining the same behavior.
🤖 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 `@src/components/parameters-tabs.tsx`:
- Around line 448-454: The NetworkVisualizationParametersInline component is
receiving a userProfile prop but expects a prop named user according to its type
definition. Locate the NetworkVisualizationParametersInline component invocation
and rename the userProfile prop to user, changing userProfile={userProfile} to
user={userProfile} to ensure the component receives the user prop correctly.
- Around line 99-103: The custom equality function in the useSelector hook for
userProfile contains a redundant check for the `profile` field which does not
exist on the UserProfile type, and only checks four specific fields, missing
other OIDC standard claims that could change. Fix this by removing the
`a?.profile === b?.profile` check from the equality comparison, and either
remove the custom equality function entirely to rely on default shallow
comparison, or replace it with logic that properly compares the complete
userProfile object including all OIDC claims that should trigger re-renders.
In `@src/index.tsx`:
- Line 15: The pathname check using endsWith is too permissive and will match
any path ending with the suffix, potentially matching unintended routes. Replace
the endsWith comparison in the window.location.pathname condition with an exact
path match using strict equality (===) to ensure only the exact route
'/silent-renew-callback' triggers the silent-renew-callback component, not other
routes that happen to end with that string.
In `@src/services/utils.ts`:
- Around line 54-61: The fetchIdpSettings() function does not handle errors when
calling localStorage.setItem(), which throws a DOMException in private browsing
mode or restrictive environments, causing the promise to reject. Wrap the
localStorage.setItem call in a try-catch block within the second .then() handler
to gracefully handle the DOMException. In the catch block, log the error but
continue execution and still return the settings, ensuring the function
completes successfully even when localStorage is unavailable.
- Around line 65-75: The getCachedIdpSettings function's try-catch block only
wraps the JSON.parse call, but localStorage.getItem can throw errors in
restrictive environments. Move the localStorage.getItem(IDP_SETTINGS_CACHE_KEY)
call inside the try-catch block so that any exceptions from getItem are also
caught and handled. This ensures the function gracefully falls back to
fetchIdpSettings() when localStorage is unavailable or throws an error, not just
when the JSON parsing fails.
---
Nitpick comments:
In `@src/components/silent-renew-app.tsx`:
- Around line 30-34: The null check on userManager inside the
handleSilentRenewCallbackClosure callback is redundant because userManager is
included in the dependency array, ensuring the callback is only recreated when
userManager changes and will always be defined when the callback executes.
Remove the if (userManager) condition and directly call
handleSilentRenewCallback(userManager) to simplify the code while maintaining
the same behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08c7c860-f8c1-4fdc-ae75-d69a47f8e052
📒 Files selected for processing (7)
index.htmlsrc/components/parameters-tabs.tsxsrc/components/silent-renew-app.tsxsrc/index.jsxsrc/index.tsxsrc/services/utils.tssrc/vite-env.d.ts
💤 Files with no reviewable changes (1)
- src/index.jsx
| const userProfile = useSelector( | ||
| (state: AppState) => state.user?.profile ?? null, | ||
| (a, b) => | ||
| a === b || (a?.sub === b?.sub && a?.name === b?.name && a?.email === b?.email && a?.profile === b?.profile) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the profile type definition and usage to verify field coverage
# Search for user profile type definitions
echo "=== User profile type definitions ==="
rg -n -C3 --type=ts --type=tsx 'interface.*Profile|type.*Profile' -g '!node_modules'
# Search for where state.user is defined in Redux
echo -e "\n=== Redux user state definition ==="
rg -n -C5 'user.*:.*\{' src/redux src/store -g '*.ts' -g '*.tsx'
# Check for profile field access patterns
echo -e "\n=== Profile field access patterns ==="
rg -n 'user\.profile\.' src/components src/hooks -g '*.ts' -g '*.tsx' | head -20Repository: gridsuite/gridstudy-app
Length of output: 277
🏁 Script executed:
# First, let's look at the file structure and find Redux/state definitions
git ls-files | grep -E '(redux|store|state)' | head -20Repository: gridsuite/gridstudy-app
Length of output: 1437
🏁 Script executed:
# Examine the parameters-tabs.tsx file to understand context
head -110 src/components/parameters-tabs.tsx | tail -20Repository: gridsuite/gridstudy-app
Length of output: 1342
🏁 Script executed:
# Search for Profile type definitions more broadly
rg -n 'interface.*Profile|type.*Profile' src/ --max-count=20Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Find Redux user state definition
rg -n 'const.*user.*=|initialState.*user' src/ -A 3 | head -40Repository: gridsuite/gridstudy-app
Length of output: 3157
🏁 Script executed:
# Look at Redux reducer type definitions
head -200 src/redux/reducer.type.ts | tail -150Repository: gridsuite/gridstudy-app
Length of output: 4871
🏁 Script executed:
# Search for Profile type definition
rg -n 'interface|type' src/redux/reducer.type.ts -A 5 | head -50Repository: gridsuite/gridstudy-app
Length of output: 2144
🏁 Script executed:
# Check for userProfile usage in the file and related hooks
rg -n 'useParametersBackend' src/components/ -A 2 | head -30Repository: gridsuite/gridstudy-app
Length of output: 1807
🏁 Script executed:
# Search for UserProfile type import/definition
rg -n 'UserProfile|export.*Profile' src/ -B 2 -A 2 | head -40Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Look at the Redux reducer to see user state structure
cat src/redux/reducer.ts | head -100Repository: gridsuite/gridstudy-app
Length of output: 3388
🏁 Script executed:
# Search more broadly for user profile initialization
rg -n 'user.*profile|profile.*sub' src/redux/ -B 2 -A 2Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Check what useParametersBackend expects in external library or docs
rg -n 'useParametersBackend' src/ -B 5 -A 5 | head -60Repository: gridsuite/gridstudy-app
Length of output: 4116
🏁 Script executed:
# Look at how NetworkVisualizationParametersInline uses userProfile
rg -n 'NetworkVisualizationParametersInline' src/components/parameters-tabs.tsx -B 3 -A 3Repository: gridsuite/gridstudy-app
Length of output: 653
🏁 Script executed:
# Find where user state is set/populated
rg -n 'state.user|USER_ACTION|UserAction' src/redux/reducer.ts -A 10 | head -80Repository: gridsuite/gridstudy-app
Length of output: 1352
🏁 Script executed:
# Check for any type definition files related to User
fd -name '*.ts' -path '*/redux/*' | xargs rg -l 'user|User' | head -20Repository: gridsuite/gridstudy-app
Length of output: 1020
🏁 Script executed:
# Look for user profile shape in entire codebase
rg -n 'profile:\s*\{|profile.*:' src/ -B 2 -A 5 | head -60Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Look at reducer.type.ts to see AppState definition
cat src/redux/reducer.type.ts | grep -A 30 'export.*AppState\|user.*:'Repository: gridsuite/gridstudy-app
Length of output: 1423
🏁 Script executed:
# Check if UserProfile is imported or defined anywhere
rg -n 'UserProfile|CommonStoreState' src/ -B 1 -A 3 | head -50Repository: gridsuite/gridstudy-app
Length of output: 674
🏁 Script executed:
# Look at package.json to understand commons-ui version and what it provides
cat package.json | grep -A 5 -B 5 'commons-ui'Repository: gridsuite/gridstudy-app
Length of output: 436
🏁 Script executed:
# Search for CommonStoreState definition in node_modules or see if it's documented
find . -name 'commons-ui*' -o -name 'node_modules/@gridsuite/commons-ui' 2>/dev/null | head -5Repository: gridsuite/gridstudy-app
Length of output: 104
🏁 Script executed:
# Check if there are any type imports from commons-ui that reveal UserProfile structure
rg -n 'from.*commons-ui' src/ -A 5 | grep -i 'user\|profile' | head -30Repository: gridsuite/gridstudy-app
Length of output: 3078
🏁 Script executed:
# Look for comments or documentation explaining the custom equality function
rg -n 'equality|userProfile' src/components/parameters-tabs.tsx -B 5 -A 2Repository: gridsuite/gridstudy-app
Length of output: 3263
🏁 Script executed:
# Check the app.jsx file which has the same selector to see if there's more context
sed -n '85,110p' src/components/app.jsxRepository: gridsuite/gridstudy-app
Length of output: 1068
🏁 Script executed:
# Check the commons-ui type definitions
cat node_modules/@gridsuite/commons-ui/package.json | grep '"main"\|"types"'Repository: gridsuite/gridstudy-app
Length of output: 190
🏁 Script executed:
# Look at the actual UserProfile or CommonStoreState types in commons-ui
find node_modules/@gridsuite/commons-ui -name '*.d.ts' | xargs grep -l 'CommonStoreState\|UserProfile' | head -5Repository: gridsuite/gridstudy-app
Length of output: 485
🏁 Script executed:
# Check what fields are actually on the user profile by looking at actual usage
rg -n 'userProfile\.' src/components/ -B 2 -A 1 | head -40Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Find and examine CommonStoreState type definition
cat node_modules/@gridsuite/commons-ui/dist/index.d.ts | grep -A 20 'CommonStoreState'Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Check what UserProfile looks like in the type definitions
cat node_modules/@gridsuite/commons-ui/dist/index.d.ts | grep -B 5 -A 15 'interface.*User\|type.*User'Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Look for any other fields accessed on user or profile objects in the codebase
rg -n 'user\?.profile\?\.([a-zA-Z]+)|profile\.([a-zA-Z]+)' src/ | head -30Repository: gridsuite/gridstudy-app
Length of output: 428
🏁 Script executed:
# Look at the actual index.d.ts file to find the user/profile types
head -500 node_modules/@gridsuite/commons-ui/dist/index.d.ts | tail -400Repository: gridsuite/gridstudy-app
Length of output: 611
🏁 Script executed:
# Search for where 'user' is used in type definitions
grep -n "user\|User\|profile\|Profile" node_modules/@gridsuite/commons-ui/dist/index.d.ts | head -50Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Check all accesses to profile properties in the codebase
rg -n '\\.profile\\.|\\.profile\\?' src/ --max-count=20Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Look for the redux exports in commons-ui
cat node_modules/@gridsuite/commons-ui/dist/redux/index.d.ts | head -100Repository: gridsuite/gridstudy-app
Length of output: 402
🏁 Script executed:
# Check if there's a store or state type definition
find node_modules/@gridsuite/commons-ui/dist -name '*.d.ts' -exec grep -l 'CommonStoreState\|user.*:' {} \; | head -10Repository: gridsuite/gridstudy-app
Length of output: 1716
🏁 Script executed:
# Try to find where user profile is defined
find node_modules/@gridsuite/commons-ui/dist -name '*.d.ts' -exec grep -H "user.*profile\|sub.*email" {} \; | head -20Repository: gridsuite/gridstudy-app
Length of output: 1027
🏁 Script executed:
# Check the commonStore type definitions
cat node_modules/@gridsuite/commons-ui/dist/redux/commonStore.d.tsRepository: gridsuite/gridstudy-app
Length of output: 821
🏁 Script executed:
# Also check the actual source if available
find node_modules/@gridsuite/commons-ui -name 'commonStore.ts' -o -name 'commonStore.tsx' 2>/dev/null | xargs cat 2>/dev/null | head -150Repository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
# Check the oidc-client-ts User type
cat node_modules/oidc-client-ts/dist/User.d.ts | head -100Repository: gridsuite/gridstudy-app
Length of output: 142
🏁 Script executed:
# Also check if there are any other profile fields being used
rg -n 'profile\.[a-zA-Z_]' src/ -o | sort -uRepository: gridsuite/gridstudy-app
Length of output: 233
🏁 Script executed:
# Look at what fields are actually on the profile by checking all accesses
rg 'user\.profile\.|profile\?\.' src/ -o -h | sort -uRepository: gridsuite/gridstudy-app
Length of output: 9343
🏁 Script executed:
# Try to find oidc-client-ts in node_modules
find node_modules -name 'oidc-client*' -type d | head -5Repository: gridsuite/gridstudy-app
Length of output: 95
🏁 Script executed:
# Check what other profile-related fields might exist in the codebase
rg 'profile\[' src/ -h -o | sort -uRepository: gridsuite/gridstudy-app
Length of output: 9343
🏁 Script executed:
# Look at useParametersBackend calls to see what fields from userProfile are actually used
grep -A 20 'useParametersBackend' src/components/parameters-tabs.tsx | head -50Repository: gridsuite/gridstudy-app
Length of output: 2231
🏁 Script executed:
# Find the User type definition from oidc-client-ts
find node_modules/oidc-client-ts -name '*.d.ts' | xargs cat 2>/dev/null | grep -A 30 'export.*User\|interface User' | head -100Repository: gridsuite/gridstudy-app
Length of output: 3785
🏁 Script executed:
# Check for package.json in oidc-client-ts to understand what we're working with
cat node_modules/oidc-client-ts/package.json | grep '"name"\|"version"\|"main"\|"types"'Repository: gridsuite/gridstudy-app
Length of output: 253
🏁 Script executed:
# Find the UserProfile type definition
find node_modules/oidc-client-ts -name '*.d.ts' | xargs grep -A 20 'interface UserProfile\|export.*UserProfile' | head -80Repository: gridsuite/gridstudy-app
Length of output: 555
🏁 Script executed:
# Find the IdTokenClaims type definition
find node_modules/oidc-client-ts -name '*.d.ts' | xargs grep -B 5 -A 30 'interface IdTokenClaims\|type IdTokenClaims' | head -100Repository: gridsuite/gridstudy-app
Length of output: 4167
Remove or fix the redundant profile field check in the custom equality function.
The equality function checks four fields (sub, name, email, profile), but according to the oidc-client-ts type definitions, UserProfile is an alias for IdTokenClaims, which is an extended interface that can include many additional fields such as auth_time, nonce, acr, amr, azp, sid, and any custom claims. The check a?.profile === b?.profile appears redundant since profile is not a property of UserProfile itself.
Additionally, changes to any OIDC standard claims beyond the four checked fields (e.g., auth_time, custom attributes added by the identity provider) would not trigger re-renders, potentially resulting in stale data being displayed.
🤖 Prompt for 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.
In `@src/components/parameters-tabs.tsx` around lines 99 - 103, The custom
equality function in the useSelector hook for userProfile contains a redundant
check for the `profile` field which does not exist on the UserProfile type, and
only checks four specific fields, missing other OIDC standard claims that could
change. Fix this by removing the `a?.profile === b?.profile` check from the
equality comparison, and either remove the custom equality function entirely to
rely on default shallow comparison, or replace it with logic that properly
compares the complete userProfile object including all OIDC claims that should
trigger re-renders.
| <NetworkVisualizationParametersInline | ||
| studyUuid={studyUuid} | ||
| setHaveDirtyFields={setDirtyFields} | ||
| user={user} | ||
| userProfile={userProfile} | ||
| parameters={networkVisualizationsParameters} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check NetworkVisualizationParametersInline prop interface
# Find the component definition and its props interface
echo "=== NetworkVisualizationParametersInline component definition ==="
ast-grep --pattern $'interface $PROPS {
$$$
}
$$$NetworkVisualizationParametersInline$$$'
# Alternative: search for the component definition directly
rg -n -C10 'NetworkVisualizationParametersInline.*FunctionComponent|NetworkVisualizationParametersInline.*FC' src/components -g '*.tsx' -g '*.ts'
# Check for any usage of user vs userProfile in the component
echo -e "\n=== User/userProfile prop usage ==="
rg -n 'props\.(user|userProfile)|\\{.*user.*\\}.*=.*props' src/components/network-visualization -g '*.tsx' -g '*.ts' | head -20Repository: gridsuite/gridstudy-app
Length of output: 350
🏁 Script executed:
#!/bin/bash
# Find files containing NetworkVisualizationParametersInline component
echo "=== Finding component definition file ==="
rg -l 'NetworkVisualizationParametersInline' src/components
# Once found, check the component's props interface
echo -e "\n=== Searching for component props interface ==="
rg -n -B5 -A20 'NetworkVisualizationParametersInline.*=' src/componentsRepository: gridsuite/gridstudy-app
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Search for the actual component definition across the entire src/components
echo "=== Searching for NetworkVisualizationParametersInline definition ==="
rg -n 'export.*NetworkVisualizationParametersInline' src/components
# Also search for the component name in imports to find where it's exported from
echo -e "\n=== Finding imports of the component ==="
rg -n 'from.*NetworkVisualizationParametersInline|import.*NetworkVisualizationParametersInline' src/components
# List directory structure to find network-visualization files
echo -e "\n=== Network visualization related files ==="
fd 'network' src/components -t fRepository: gridsuite/gridstudy-app
Length of output: 2746
🏁 Script executed:
#!/bin/bash
# Search across entire repository for NetworkVisualizationParametersInline
echo "=== Searching entire codebase for NetworkVisualizationParametersInline ==="
rg -n 'NetworkVisualizationParametersInline' . --type ts --type tsx
# Check if it might be from an external library or package
echo -e "\n=== Checking package.json for visualization libraries ==="
grep -i "visual\|network" package.json | head -20Repository: gridsuite/gridstudy-app
Length of output: 328
🏁 Script executed:
#!/bin/bash
# Search without file type restrictions
echo "=== Searching for NetworkVisualizationParametersInline ==="
rg 'NetworkVisualizationParametersInline' .
# Also check parameters-tabs.tsx directly to see the import
echo -e "\n=== Check imports in parameters-tabs.tsx ==="
head -100 src/components/parameters-tabs.tsx | grep -n "import\|NetworkVisualization"Repository: gridsuite/gridstudy-app
Length of output: 2496
🏁 Script executed:
#!/bin/bash
# Get the full import statement for NetworkVisualizationParametersInline
echo "=== Full import statement ==="
sed -n '31,50p' src/components/parameters-tabs.tsx
# Now search for where NetworkVisualizationParametersInline is defined
echo -e "\n=== Search for the component definition in dialogs/parameters ==="
rg -n 'NetworkVisualizationParametersInline' src/components/dialogs/parameters
# List files in that directory
echo -e "\n=== Files in dialogs/parameters directory ==="
find src/components/dialogs/parameters -name "*.tsx" -o -name "*.ts" | head -30Repository: gridsuite/gridstudy-app
Length of output: 1788
🏁 Script executed:
#!/bin/bash
# Get a wider context to find the complete import source
echo "=== Get complete import block ==="
sed -n '31,60p' src/components/parameters-tabs.tsx
# Search for `@powsybl` imports
echo -e "\n=== Check for `@powsybl` imports ==="
rg -n '`@powsybl`' src/components/parameters-tabs.tsx | head -10Repository: gridsuite/gridstudy-app
Length of output: 1231
🏁 Script executed:
#!/bin/bash
# Now let's look at the actual usage at lines 448-454
echo "=== Code at lines 448-454 ==="
sed -n '448,454p' src/components/parameters-tabs.tsx
# Check the package.json for `@gridsuite/commons-ui` version
echo -e "\n=== Check commons-ui version ==="
grep -A2 -B2 'commons-ui' package.json
# Search for any type definitions or interfaces related to NetworkVisualizationParametersInline in the repo
echo -e "\n=== Search for type definitions ==="
rg -n 'userProfile|user.*Props' src/components/parameters-tabs.tsx | grep -i networkRepository: gridsuite/gridstudy-app
Length of output: 695
🏁 Script executed:
#!/bin/bash
# Let's check if there's a node_modules or type definition available
echo "=== Check for commons-ui types ==="
find . -name "`@gridsuite`" -type d 2>/dev/null | head -5
# Also check the broader context around the usage to understand the code flow
echo -e "\n=== Full context around lines 440-460 ==="
sed -n '440,460p' src/components/parameters-tabs.tsx
# Check if userProfile is defined and used elsewhere
echo -e "\n=== Usage of userProfile in parameters-tabs.tsx ==="
rg -n 'userProfile' src/components/parameters-tabs.tsx | head -20Repository: gridsuite/gridstudy-app
Length of output: 1297
🏁 Script executed:
#!/bin/bash
# Let's inspect the commons-ui package to find the component definition
echo "=== Looking for NetworkVisualizationParametersInline in commons-ui ==="
find ./node_modules/@gridsuite/commons-ui -name "*.d.ts" -o -name "index.ts" | head -10
# Search for the component in the commons-ui distribution
echo -e "\n=== Search in commons-ui for NetworkVisualizationParametersInline ==="
rg -n 'NetworkVisualizationParametersInline' ./node_modules/@gridsuite/commons-ui 2>/dev/null | head -20
# Try to find type definitions
echo -e "\n=== Find component interface in commons-ui ==="
rg -n 'interface.*NetworkVisualization|type.*NetworkVisualization' ./node_modules/@gridsuite/commons-ui 2>/dev/null | head -20Repository: gridsuite/gridstudy-app
Length of output: 3764
🏁 Script executed:
#!/bin/bash
# Get the full type definition
echo "=== Full type definition of NetworkVisualizationParametersInline ==="
cat ./node_modules/@gridsuite/commons-ui/dist/features/parameters/network-visualizations/network-visualizations-parameters-inline.d.tsRepository: gridsuite/gridstudy-app
Length of output: 622
Change userProfile prop to user.
The NetworkVisualizationParametersInline component expects a prop named user, not userProfile. The component's type definition from @gridsuite/commons-ui (v0.227.0) declares:
user: User | null;
Passing userProfile will result in the user prop being undefined in the component.
🤖 Prompt for 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.
In `@src/components/parameters-tabs.tsx` around lines 448 - 454, The
NetworkVisualizationParametersInline component is receiving a userProfile prop
but expects a prop named user according to its type definition. Locate the
NetworkVisualizationParametersInline component invocation and rename the
userProfile prop to user, changing userProfile={userProfile} to
user={userProfile} to ensure the component receives the user prop correctly.
| const root = createRoot(container!); | ||
|
|
||
| async function renderApp() { | ||
| if (window.location.pathname.endsWith('/silent-renew-callback')) { |
There was a problem hiding this comment.
Use exact path matching instead of endsWith.
pathname.endsWith('/silent-renew-callback') will match any path ending with that suffix, including unintended routes like /admin/silent-renew-callback or /user/silent-renew-callback. This could cause those routes to incorrectly render the silent-renew component instead of the intended content.
🔧 Proposed fix using exact match or prefix check
Option 1: Exact match (if no base path)
-if (window.location.pathname.endsWith('/silent-renew-callback')) {
+if (window.location.pathname === '/silent-renew-callback') {
root.render(<SilentRenewApp />);
return;
}Option 2: Base path aware (if app may be deployed under a base path)
+const basePath = new URL(document.baseURI).pathname;
+const relativePath = window.location.pathname.slice(basePath.length - 1);
-if (window.location.pathname.endsWith('/silent-renew-callback')) {
+if (relativePath === '/silent-renew-callback') {
root.render(<SilentRenewApp />);
return;
}📝 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.
| if (window.location.pathname.endsWith('/silent-renew-callback')) { | |
| if (window.location.pathname === '/silent-renew-callback') { | |
| root.render(<SilentRenewApp />); | |
| return; | |
| } |
🤖 Prompt for 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.
In `@src/index.tsx` at line 15, The pathname check using endsWith is too
permissive and will match any path ending with the suffix, potentially matching
unintended routes. Replace the endsWith comparison in the
window.location.pathname condition with an exact path match using strict
equality (===) to ensure only the exact route '/silent-renew-callback' triggers
the silent-renew-callback component, not other routes that happen to end with
that string.
| export function fetchIdpSettings(): Promise<IdpSettings> { | ||
| return fetch('idpSettings.json') | ||
| .then((res) => res.json()) | ||
| .then((settings: IdpSettings) => { | ||
| localStorage.setItem(IDP_SETTINGS_CACHE_KEY, JSON.stringify(settings)); | ||
| return settings; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Guard against localStorage unavailability.
In private browsing mode or restrictive browser environments, localStorage.setItem throws a DOMException. This will cause fetchIdpSettings() to reject and break both the main application bootstrap and the silent-renew flow.
🛡️ Proposed fix to add try-catch around localStorage operations
export function fetchIdpSettings(): Promise<IdpSettings> {
return fetch('idpSettings.json')
.then((res) => res.json())
.then((settings: IdpSettings) => {
- localStorage.setItem(IDP_SETTINGS_CACHE_KEY, JSON.stringify(settings));
+ try {
+ localStorage.setItem(IDP_SETTINGS_CACHE_KEY, JSON.stringify(settings));
+ } catch (e) {
+ console.warn('Failed to cache IdP settings:', e);
+ }
return settings;
});
}📝 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.
| export function fetchIdpSettings(): Promise<IdpSettings> { | |
| return fetch('idpSettings.json') | |
| .then((res) => res.json()) | |
| .then((settings: IdpSettings) => { | |
| localStorage.setItem(IDP_SETTINGS_CACHE_KEY, JSON.stringify(settings)); | |
| return settings; | |
| }); | |
| } | |
| export function fetchIdpSettings(): Promise<IdpSettings> { | |
| return fetch('idpSettings.json') | |
| .then((res) => res.json()) | |
| .then((settings: IdpSettings) => { | |
| try { | |
| localStorage.setItem(IDP_SETTINGS_CACHE_KEY, JSON.stringify(settings)); | |
| } catch (e) { | |
| console.warn('Failed to cache IdP settings:', e); | |
| } | |
| return settings; | |
| }); | |
| } |
🤖 Prompt for 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.
In `@src/services/utils.ts` around lines 54 - 61, The fetchIdpSettings() function
does not handle errors when calling localStorage.setItem(), which throws a
DOMException in private browsing mode or restrictive environments, causing the
promise to reject. Wrap the localStorage.setItem call in a try-catch block
within the second .then() handler to gracefully handle the DOMException. In the
catch block, log the error but continue execution and still return the settings,
ensuring the function completes successfully even when localStorage is
unavailable.
| export function getCachedIdpSettings(): Promise<IdpSettings> { | ||
| const cached = localStorage.getItem(IDP_SETTINGS_CACHE_KEY); | ||
| if (cached) { | ||
| try { | ||
| return Promise.resolve(JSON.parse(cached) as IdpSettings); | ||
| } catch { | ||
| // corrupted cache -> fall back to a fresh fetch | ||
| } | ||
| } | ||
| return fetchIdpSettings(); | ||
| } |
There was a problem hiding this comment.
Guard against localStorage unavailability in getCachedIdpSettings.
localStorage.getItem can throw in restrictive environments. The try-catch on line 68-72 only wraps JSON.parse, not the getItem call itself.
🛡️ Proposed fix to wrap getItem in try-catch
export function getCachedIdpSettings(): Promise<IdpSettings> {
+ try {
- const cached = localStorage.getItem(IDP_SETTINGS_CACHE_KEY);
- if (cached) {
- try {
- return Promise.resolve(JSON.parse(cached) as IdpSettings);
- } catch {
- // corrupted cache -> fall back to a fresh fetch
- }
- }
+ const cached = localStorage.getItem(IDP_SETTINGS_CACHE_KEY);
+ if (cached) {
+ return Promise.resolve(JSON.parse(cached) as IdpSettings);
+ }
+ } catch (e) {
+ // localStorage unavailable or cache corrupted -> fall back to fresh fetch
+ console.warn('Failed to read cached IdP settings:', e);
+ }
return fetchIdpSettings();
}🤖 Prompt for 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.
In `@src/services/utils.ts` around lines 65 - 75, The getCachedIdpSettings
function's try-catch block only wraps the JSON.parse call, but
localStorage.getItem can throw errors in restrictive environments. Move the
localStorage.getItem(IDP_SETTINGS_CACHE_KEY) call inside the try-catch block so
that any exceptions from getItem are also caught and handled. This ensures the
function gracefully falls back to fetchIdpSettings() when localStorage is
unavailable or throws an error, not just when the JSON parsing fails.
1 participant
PR Summary
Context
On every OIDC silent renew, oidc-client-ts loads
/silent-renew-callbackinside ahidden iframe. Because
index.tsxalways rendered<AppWrapper />, the entireapplication was bootstrapped inside that iframe just to end up calling
signinSilentCallback(): Redux store,IntlProviderand all translations,AG Grid registration, MUI theme,
SnackbarProviderandNotificationsProvider.NotificationsProvideradditionally fetchedenv.jsonand opened a notificationWebSocket on each renewal — all of it pure overhead repeated at every token refresh.
Changes
index.tsx: branch on the pathname before rendering. On/silent-renew-callbackrender a new lightweight
SilentRenewApp; otherwise render<AppWrapper />as before.components/silent-renew-app.tsx: rebuilds theUserManagerviainitializeAuthenticationProd(..., isSilentRenew=true, ...)(same settings, samelocalStoragestate store, same Azure authority hack as the parent) and completesthe flow through commons-ui's
SilentRenewCallbackHandler.utils/rest-api.ts:fetchIdpSettingsnow caches the result inlocalStorage;added
getCachedIdpSettings, used only by the silent-renew path, which reads thatcache (no network) and falls back to a real fetch if it is missing/corrupted.
Result
Inside the silent-renew iframe, requests go from a full SPA boot
(
idpSettings.json+env.json+ WebSocket + app bundle execution) down to asingle
POST /token— the actual token exchange. No more spurious WebSocketreconnections triggered by the renewal iframe.
Testing
env.json, no longerre-fetches
idpSettings.json, and does not open a WebSocket — onlyPOST /tokenremains.notification WebSocket is not reconnected on each cycle.