Refactor lint hotspots and align frontend/backend ESLint guardrails#2856
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnforces stricter ESLint rules in backend and frontend and applies code changes across multiple files to comply (brace style, multiline ternaries, no nested ternaries, curly all, max-depth, max-nested-callbacks). Various functions were refactored or made explicit to satisfy these rules. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
frontend/src/lib/xterm-addon-k8s-attach.js (1)
101-102: Consider including the channel value in the error message for easier debugging.When an unsupported channel is received, knowing the actual channel value would help diagnose issues.
💡 Suggested improvement
default: - throw Error('Unsupported message!') + throw Error(`Unsupported channel: ${channel}`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/xterm-addon-k8s-attach.js` around lines 101 - 102, The default case that throws Error('Unsupported message!') should include the actual channel value to aid debugging; update the default branch in the switch handling incoming channels (the branch that currently calls throw Error('Unsupported message!')) to construct the error message with the channel (e.g., include channel or msg.channel/packet value via string interpolation or JSON.stringify) so logs show "Unsupported message: <channel-value>".frontend/src/router/guards.js (1)
23-27: Remove stale'Credential'entry fromcredentialRoutes.
frontend/src/router/routes.jsregistersname: 'Credentials'(plural), not'Credential'. Keeping the singular value leaves dead matching logic and can hide route-name typos later.Proposed cleanup
-const credentialRoutes = new Set(['Credentials', 'Credential']) +const credentialRoutes = new Set(['Credentials'])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/router/guards.js` around lines 23 - 27, Remove the stale 'Credential' entry from the credentialRoutes Set to match the actual route name registration; update the credentialRoutes declaration (the Set referenced as credentialRoutes) to only contain 'Credentials' so route-name matching logic doesn't include a dead/incorrect 'Credential' value.frontend/__fixtures__/credentials.js (2)
7-12:withQuotasmutates the input object.The function modifies
resourcein-place by assigningresource.quotas. This works correctly but could be surprising since the function name and return value suggest it might return a new object. For fixture code this is acceptable, but an immutable approach would be cleaner.♻️ Optional: Immutable alternative
function withQuotas (resource, quotas) { - if (resource && quotas.length > 0) { - resource.quotas = quotas.map(({ metadata }) => metadata) + if (!resource) { + return resource } - return resource + if (quotas.length > 0) { + return { + ...resource, + quotas: quotas.map(({ metadata }) => metadata), + } + } + return resource }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__fixtures__/credentials.js` around lines 7 - 12, withQuotas currently mutates the passed-in resource by assigning resource.quotas; change it to be immutable by returning a new object when quotas are present (e.g., create and return {...resource, quotas: quotas.map(({metadata}) => metadata)}) and otherwise return the original resource unchanged, so the function no longer mutates the input resource while still mapping metadata into quotas.
14-29: Consider fixing thesecretNamepacetypo.The parameter
secretNamepace(lines 14, 19, 26) should besecretNamespace. This typo is propagated from existing code (line 122) into the new helper functions. While maintaining consistency with the existing codebase, this would be a good opportunity to fix the typo across the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/__fixtures__/credentials.js` around lines 14 - 29, The helper function getCredentialsRef uses the misspelled parameter and variable secretNamepace; rename that parameter to secretNamespace in the function signature and replace all uses inside getCredentialsRef (the namespace: field in both returned objects) to use secretNamespace, and then update any callers in this file that pass or reference secretNamepace (including the other helper(s) around line 122) to use secretNamespace so the name is consistent across the file.frontend/src/composables/credential/helper.js (1)
53-56: Consider adding defensive checks to avoid relying on exception handling.If
serviceaccount.jsonis missing or contains invalid JSON,JSON.parsewill throw. While thetry/catchinsecretDetailshandles this, relying on exceptions for control flow can mask other unexpected errors.♻️ Proposed defensive implementation
function getGcpProjectId (secretData) { const serviceAccount = get(secretData, ['serviceaccount.json']) + if (!serviceAccount) { + return undefined + } - return get(JSON.parse(decodeBase64(serviceAccount)), ['project_id']) + const decoded = decodeBase64(serviceAccount) + if (!decoded) { + return undefined + } + return get(JSON.parse(decoded), ['project_id']) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/credential/helper.js` around lines 53 - 56, The getGcpProjectId function currently assumes serviceaccount.json exists and is valid JSON; update getGcpProjectId to defensively guard against missing or invalid data by first checking that serviceAccount (from get(secretData, ['serviceaccount.json'])) is truthy, then decodeBase64 only if present, and perform a safe JSON parse for the decoded string (catch parse errors locally and return undefined/null) before reading ['project_id']; limit try/catch to the JSON.parse step so other errors bubble and ensure the function returns a safe fallback when serviceAccount is absent or malformed.frontend/src/composables/useSocketEventHandler.js (1)
65-74: Clean refactoring that reduces complexity.The extraction successfully flattens the control flow and improves readability. The logic correctly preserves the original behavior: Status items are logged and delegated to the 404 handler, while regular items store the full object in
uidMap(as required by downstreamoperator.set()per the shoot store context).Optional: Consider defensive null check.
Line 72 accesses
item.metadata.uiddirectly. Ifmetadatais undefined, this throws. Items fromsocketStore.synchronizeshould always have metadata, and downstream code also expects it, so this is likely acceptable. However, for defensive programming:💡 Optional: add null guard
function synchronizeItem (item, store, uidMap) { if (item.kind === 'Status') { logger.info('Failed to synchronize a single %s: %s', store.$id, item.message) markDeletedUidForStatusItem(item, uidMap) return } - const uid = item.metadata.uid + const uid = item.metadata?.uid + if (!uid) { + logger.warn('Received item without uid for %s', store.$id) + return + } uidMap.set(uid, item) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/useSocketEventHandler.js` around lines 65 - 74, The function synchronizeItem accesses item.metadata.uid directly which can throw if metadata is undefined; update synchronizeItem to defensively check metadata (and uid) before using it — e.g., in the synchronizeItem function, after the Status branch, verify item.metadata and item.metadata.uid exist, log an informative error via logger if missing and return (or otherwise handle gracefully) instead of dereferencing, ensuring uidMap.set is only called with a valid uid and preserving existing use of markDeletedUidForStatusItem for Status items.frontend/src/store/shoot/helper.js (2)
278-279: Avoid rebuilding resolver maps on everygetRawValcall.Line 278 creates a fresh resolver object for each lookup. In hot paths (search/sort across many rows), this adds unnecessary allocations. Prefer a static resolver map with
(context, item)parameters.Refactor sketch
+const rawValueResolvers = { + purpose: ({ item }) => get(item.spec, ['purpose']), + lastOperation: ({ item }) => get(item, ['status', 'lastOperation']), + createdAt: ({ item }) => item.metadata.creationTimestamp, + createdBy: ({ item }) => getCreatedBy(item.metadata), + project: ({ context, item }) => getProjectValue(context, item), + k8sVersion: ({ item }) => get(item.spec, ['kubernetes', 'version']), + infrastructure: ({ item }) => `${get(item.spec, ['provider', 'type'])} ${get(item.spec, ['region'])}`, + seed: ({ item }) => get(item, ['spec', 'seedName']), + ticketLabels: ({ context, item }) => getTicketLabelsValue(context, item), + errorCodes: ({ item }) => join(errorCodesFromArray(get(item, ['status', 'lastErrors'], [])), ' '), + controlPlaneHighAvailability: ({ item }) => get(item.spec, ['controlPlane', 'highAvailability', 'failureTolerance', 'type']), + issueSince: ({ item }) => getIssueSince(item.status), + technicalId: ({ item }) => item.status?.technicalID, + workers: ({ item }) => item.spec.provider.workers?.length ?? 0, +} + export function getRawVal (context, item, column) { - const resolvers = createRawValueResolvers(context, item) - const resolver = get(resolvers, [column]) + const resolver = get(rawValueResolvers, [column]) if (resolver) { - return resolver() + return resolver({ context, item }) } if (isCustomField(column)) { return getCustomFieldValue(context, item, column) } return get(item.metadata, [column]) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/store/shoot/helper.js` around lines 278 - 279, The current code calls createRawValueResolvers(context, item) inside getRawVal and rebuilds the entire resolver map per lookup; instead, refactor so createRawValueResolvers returns a static map of resolver functions that accept (context, item) and build that map once (module- or parent-scope) or memoize it by context, then in getRawVal call resolver = get(resolvers, [column]) and invoke resolver(context, item). Update the createRawValueResolvers signature/implementation to produce column-keyed functions (rather than bound values) and change usages of resolvers/get(resolvers, [column]) accordingly to avoid per-call allocations.
68-73: Use a shared custom-field separator to avoid search/sort vs display drift.Line 72 joins array values with
' ', while display formatting uses', 'infrontend/src/components/GShootListRow.vue(Line 419-421). Consider normalizing to one separator (or a shared constant) so displayed text and searchable/sortable raw text stay consistent.Proposed adjustment
function getCustomFieldValue (context, item, column) { const { getCustomFieldByKey } = context.shootCustomFieldsComposable const path = getCustomFieldByKey(column)?.path const value = get(item, path) - return formatValue(value, ' ') + return formatValue(value, ', ') }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/store/shoot/helper.js` around lines 68 - 73, The code in getCustomFieldValue uses a hardcoded separator ' ' when calling formatValue, causing inconsistency with the display separator ', ' used in GShootListRow; update the implementation to use a single shared separator constant (e.g., CUSTOM_FIELD_SEPARATOR) or expose a parameter from formatValue and import that constant into both getCustomFieldValue and the GShootListRow display logic so both search/sort raw text and displayed text use the same separator; locate symbols getCustomFieldValue, getCustomFieldByKey, and formatValue to apply the change and add the shared constant where formatValue and GShootListRow.vue consume it.
🤖 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/eslint.config.cjs`:
- Around line 187-202: The new ESLint rule "max-depth": ['error', 3] will break
CI because frontend/src/store/shoot/shoot.js currently has many violations (up
to depth 9); locate the offending deep nesting in functions like createOperator
(around where it’s defined in shoot.js) and refactor those functions to reduce
nesting to at most 3 (e.g., extract nested blocks into smaller helper functions,
return early to invert conditionals, or flatten nested if/else/try/catch/loops),
then re-run the linter and only add the rule once all violations in shoot.js are
fixed; do not change frontend/src/composables/useCodemirror/helper.js since it
already complies.
In `@frontend/src/components/Credentials/GSecretDialogWrapper.vue`:
- Around line 36-59: The dialogComponents lookup currently uses bracket access
with dialogComponents[this.visibleDialog] and can return prototype properties
for untrusted strings; update the component rendering logic (where
dialogComponents and this.visibleDialog are used) to first check
Object.hasOwn(dialogComponents, this.visibleDialog) and only then select
dialogComponents[this.visibleDialog], otherwise fall back to GenericDialog;
ensure this change is applied wherever dialogComponents lookup occurs (e.g., in
the render/computed method that chooses the dialog component) so prototype-chain
keys like "constructor" or "__proto__" do not get resolved.
In `@frontend/src/composables/useCodemirror/helper.js`:
- Around line 497-502: The comment inside the private method
`#warnMultipleDiscriminators` contains a typo ("cursorrently"); update the comment
to read "currently" so the method `#warnMultipleDiscriminators` (and its
parameters foundDiscriminators, parentPropertyName) has a correct inline
comment, leaving the logic and the this.logger.warn call unchanged.
---
Nitpick comments:
In `@frontend/__fixtures__/credentials.js`:
- Around line 7-12: withQuotas currently mutates the passed-in resource by
assigning resource.quotas; change it to be immutable by returning a new object
when quotas are present (e.g., create and return {...resource, quotas:
quotas.map(({metadata}) => metadata)}) and otherwise return the original
resource unchanged, so the function no longer mutates the input resource while
still mapping metadata into quotas.
- Around line 14-29: The helper function getCredentialsRef uses the misspelled
parameter and variable secretNamepace; rename that parameter to secretNamespace
in the function signature and replace all uses inside getCredentialsRef (the
namespace: field in both returned objects) to use secretNamespace, and then
update any callers in this file that pass or reference secretNamepace (including
the other helper(s) around line 122) to use secretNamespace so the name is
consistent across the file.
In `@frontend/src/composables/credential/helper.js`:
- Around line 53-56: The getGcpProjectId function currently assumes
serviceaccount.json exists and is valid JSON; update getGcpProjectId to
defensively guard against missing or invalid data by first checking that
serviceAccount (from get(secretData, ['serviceaccount.json'])) is truthy, then
decodeBase64 only if present, and perform a safe JSON parse for the decoded
string (catch parse errors locally and return undefined/null) before reading
['project_id']; limit try/catch to the JSON.parse step so other errors bubble
and ensure the function returns a safe fallback when serviceAccount is absent or
malformed.
In `@frontend/src/composables/useSocketEventHandler.js`:
- Around line 65-74: The function synchronizeItem accesses item.metadata.uid
directly which can throw if metadata is undefined; update synchronizeItem to
defensively check metadata (and uid) before using it — e.g., in the
synchronizeItem function, after the Status branch, verify item.metadata and
item.metadata.uid exist, log an informative error via logger if missing and
return (or otherwise handle gracefully) instead of dereferencing, ensuring
uidMap.set is only called with a valid uid and preserving existing use of
markDeletedUidForStatusItem for Status items.
In `@frontend/src/lib/xterm-addon-k8s-attach.js`:
- Around line 101-102: The default case that throws Error('Unsupported
message!') should include the actual channel value to aid debugging; update the
default branch in the switch handling incoming channels (the branch that
currently calls throw Error('Unsupported message!')) to construct the error
message with the channel (e.g., include channel or msg.channel/packet value via
string interpolation or JSON.stringify) so logs show "Unsupported message:
<channel-value>".
In `@frontend/src/router/guards.js`:
- Around line 23-27: Remove the stale 'Credential' entry from the
credentialRoutes Set to match the actual route name registration; update the
credentialRoutes declaration (the Set referenced as credentialRoutes) to only
contain 'Credentials' so route-name matching logic doesn't include a
dead/incorrect 'Credential' value.
In `@frontend/src/store/shoot/helper.js`:
- Around line 278-279: The current code calls createRawValueResolvers(context,
item) inside getRawVal and rebuilds the entire resolver map per lookup; instead,
refactor so createRawValueResolvers returns a static map of resolver functions
that accept (context, item) and build that map once (module- or parent-scope) or
memoize it by context, then in getRawVal call resolver = get(resolvers,
[column]) and invoke resolver(context, item). Update the createRawValueResolvers
signature/implementation to produce column-keyed functions (rather than bound
values) and change usages of resolvers/get(resolvers, [column]) accordingly to
avoid per-call allocations.
- Around line 68-73: The code in getCustomFieldValue uses a hardcoded separator
' ' when calling formatValue, causing inconsistency with the display separator
', ' used in GShootListRow; update the implementation to use a single shared
separator constant (e.g., CUSTOM_FIELD_SEPARATOR) or expose a parameter from
formatValue and import that constant into both getCustomFieldValue and the
GShootListRow display logic so both search/sort raw text and displayed text use
the same separator; locate symbols getCustomFieldValue, getCustomFieldByKey, and
formatValue to apply the change and add the shared constant where formatValue
and GShootListRow.vue consume it.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a833943c-e68a-44f2-bd64-181da2ef74a1
📒 Files selected for processing (19)
backend/__fixtures__/shoots.cjsbackend/eslint.config.cjsbackend/lib/config/gardener.jsbackend/lib/github/webhook/verify.jsbackend/lib/routes/config.jsfrontend/__fixtures__/credentials.jsfrontend/eslint.config.cjsfrontend/src/components/Credentials/GSecretDialogWrapper.vuefrontend/src/components/GShootActionRotateCredentials.vuefrontend/src/components/dialogs/GDialog.vuefrontend/src/composables/credential/helper.jsfrontend/src/composables/useCodemirror/helper.jsfrontend/src/composables/useSocketEventHandler.jsfrontend/src/composables/useTableSorting/helper.jsfrontend/src/lib/xterm-addon-k8s-attach.jsfrontend/src/router/guards.jsfrontend/src/store/shoot/helper.jsfrontend/src/utils/index.jsfrontend/vite/g-mdi-meta.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/composables/credential/helper.js (1)
97-97: Consider direct property access instead of lodashget.Using
get(detailByProviderType, [providerType])for a single-level property lookup on a plain object is unnecessary. Direct bracket notation is clearer and avoids the function call overhead.♻️ Suggested simplification
- return get(detailByProviderType, [providerType])?.() + return detailByProviderType[providerType]?.()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/composables/credential/helper.js` at line 97, The code uses lodash's get for a simple single-level lookup: replace get(detailByProviderType, [providerType])?.() with direct bracket access on the detailByProviderType object (e.g., detailByProviderType[providerType]) and then invoke the result if it's a function; update the call site referencing detailByProviderType and providerType to use direct property access to avoid the unnecessary get() call overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/composables/credential/helper.js`:
- Line 97: The code uses lodash's get for a simple single-level lookup: replace
get(detailByProviderType, [providerType])?.() with direct bracket access on the
detailByProviderType object (e.g., detailByProviderType[providerType]) and then
invoke the result if it's a function; update the call site referencing
detailByProviderType and providerType to use direct property access to avoid the
unnecessary get() call overhead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ad18d89-15d5-47b6-80a7-b5932d6b7694
📒 Files selected for processing (10)
frontend/__fixtures__/credentials.jsfrontend/src/components/Credentials/GSecretDialogWrapper.vuefrontend/src/components/GShootListRow.vuefrontend/src/composables/credential/helper.jsfrontend/src/composables/useCodemirror/helper.jsfrontend/src/composables/useProjectShootCustomFields/helper.jsfrontend/src/composables/useSocketEventHandler.jsfrontend/src/lib/xterm-addon-k8s-attach.jsfrontend/src/router/guards.jsfrontend/src/store/shoot/helper.js
✅ Files skipped from review due to trivial changes (2)
- frontend/src/composables/useCodemirror/helper.js
- frontend/src/components/Credentials/GSecretDialogWrapper.vue
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/router/guards.js
- frontend/src/store/shoot/helper.js
- frontend/fixtures/credentials.js
Enforce multiline ternary Enforce braces for if and similar statements
73537f5 to
cdc2ba3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/composables/credential/helper.js`:
- Around line 53-65: getGcpProjectId is swallowing JSON parse errors and
returning undefined, causing the gcp branch to produce a singleValueDetail with
value: undefined instead of letting secretDetails fallback to undefined; change
getGcpProjectId so it does not catch/return on JSON.parse errors (remove the
try/catch or rethrow the caught error) so malformed serviceaccount.json bubbles
up to callers, or alternatively update the gcp branch that calls getGcpProjectId
and uses singleValueDetail to skip creating the Project / singleValueDetail
entry when getGcpProjectId returns undefined; reference getGcpProjectId and the
code path that builds singleValueDetail in the gcp branch to implement one of
these fixes.
- Around line 68-98: getSecretDetailsByProviderType uses lodash/get which will
resolve inherited properties (e.g., "constructor") and then call them; guard
against prototype-chain keys by checking own properties on the dispatcher object
before invoking: in getSecretDetailsByProviderType, replace the lodash/get usage
with an own-property check on detailByProviderType (e.g.,
Object.prototype.hasOwnProperty.call(detailByProviderType, providerType)) and if
true call detailByProviderType[providerType]() else return undefined, ensuring
providerType cannot invoke inherited members.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13404448-7264-4a8e-8247-f4335b20639c
📒 Files selected for processing (12)
backend/eslint.config.cjsfrontend/__fixtures__/credentials.jsfrontend/eslint.config.cjsfrontend/src/components/Credentials/GSecretDialogWrapper.vuefrontend/src/components/GShootListRow.vuefrontend/src/composables/credential/helper.jsfrontend/src/composables/useCodemirror/helper.jsfrontend/src/composables/useProjectShootCustomFields/helper.jsfrontend/src/composables/useSocketEventHandler.jsfrontend/src/lib/xterm-addon-k8s-attach.jsfrontend/src/router/guards.jsfrontend/src/store/shoot/helper.js
✅ Files skipped from review due to trivial changes (2)
- frontend/src/composables/useProjectShootCustomFields/helper.js
- frontend/eslint.config.cjs
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/eslint.config.cjs
- frontend/src/store/shoot/helper.js
- frontend/src/lib/xterm-addon-k8s-attach.js
- frontend/src/components/Credentials/GSecretDialogWrapper.vue
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petersutter 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 |
|
LGTM label has been added. DetailsGit tree hash: c228b48e166ac6be4e1dca145de91eca0d3a0159 |
What this PR does / why we need it:
This PR refactors several frontend and backend lint hotspots to satisfy the recently tightened ESLint rules, especially around nested ternaries, max depth, and callback nesting. The changes focus on extracting helper functions, flattening control flow, and improving consistency of refactor style across the affected modules.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note:
Summary by CodeRabbit
Refactor
Style
Bug Fixes / Behavior