Skip to content

chore: fix 87 eslint warnings across the codebase#7112

Merged
isaacroldan merged 1 commit intomainfrom
03-26-chore_fix_87_eslint_warnings_across_the_codebase
Mar 27, 2026
Merged

chore: fix 87 eslint warnings across the codebase#7112
isaacroldan merged 1 commit intomainfrom
03-26-chore_fix_87_eslint_warnings_across_the_codebase

Conversation

@isaacroldan
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan commented Mar 26, 2026

What

Fix 87 out of 107 eslint warnings across the codebase. Zero behavior changes — all mechanical refactors.

Why

The lint output was noisy with warnings that obscured real issues. Cleaning them up makes CI output cleaner and establishes a healthier baseline. The remaining 20 warnings are all switch-exhaustiveness-check which require domain-specific decisions about handling new enum/union variants.

Changes

prefer-nullish-coalescing — 72 ||?? replacements

Using ?? instead of || is safer because it only coalesces null/undefined, not other falsy values like 0, "", or false. The eslint rule only flags cases where the types make this substitution safe.

One case was intentionally kept as || with a suppression comment: resolveGraphiQLKey in graphiql/server.ts where an empty string after .trim() should fall through to the derived key.

prefer-nullish-coalescing — 18 if (!x) x = yx ??= y

Replaces verbose null-guard assignment patterns with the concise nullish coalescing assignment operator. Applied to singleton patterns, lazy initialization, and default value assignments.

prefer-nullish-coalescing — 4 ternary → ??

Replaces x ? x : fallback ternaries with the equivalent x ?? fallback.

Stale eslint-disable directives — 4 removed

Removed unused suppression comments in:

  • cli-kit/src/public/node/local-storage.test.ts (2× no-explicit-any)
  • cli-kit/src/public/node/themes/api.ts (no-non-null-assertion)
  • cli/src/index.ts (specific-imports-in-bootstrap-code)
  • e2e/setup/auth.ts (no-restricted-imports)

prefer-promise-reject-errors — 2 fixed, 5 suppressed

Fixed in production code (error-handler.ts, http.ts) by wrapping rejection values in new Error().

Suppressed in update-extension.test.ts (5 occurrences) where tests intentionally reject with non-Error objects to exercise error-shape handling logic.

only-throw-error — 1 suppressed

In result.ts, the valueOrBug() method on the Err class throws this.error which is generic TError — not necessarily an Error instance. This is intentional for the Result pattern, so suppressed with a comment.

no-negated-condition — 4 fixed

Flipped if/else branches to remove negations for readability:

  • app-logs/sources.ts!isEmpty()isEmpty() with swapped branches
  • include-assets-step.ts!== undefined ternary → === undefined ternary
  • copy-source-entry.ts!== undefined=== undefined with swapped branches
  • select-app.ts!appapp with swapped branches

Before / After

Metric Before After
Total warnings 107 20
prefer-nullish-coalescing 72 1 (intentional suppression)
switch-exhaustiveness-check 20 20 (unchanged, needs domain decisions)
Stale eslint-disable 4 0
prefer-promise-reject-errors 7 0
only-throw-error 1 0
no-negated-condition 3 0

Testing

  • 507 tests passing across all 44 affected test files
  • Full pnpm lint passes with 0 errors
  • TypeScript compilation clean

Copy link
Copy Markdown
Contributor Author

isaacroldan commented Mar 26, 2026

@isaacroldan isaacroldan marked this pull request as ready for review March 26, 2026 14:59
@isaacroldan isaacroldan requested review from a team as code owners March 26, 2026 14:59
Copilot AI review requested due to automatic review settings March 26, 2026 14:59
@github-actions
Copy link
Copy Markdown
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Mechanical lint cleanups to reduce ESLint warning noise across the TypeScript/React CLI codebase, primarily by adopting nullish-coalescing patterns and removing stale ESLint suppressions.

Changes:

  • Replaced many || / if (!x) x = y patterns with ??, ??=, and related refactors to satisfy prefer-nullish-coalescing.
  • Removed stale eslint-disable directives and added targeted suppressions where non-Error throws/rejects are intentional.
  • Updated a couple of Promise.reject(...) sites in production code to always reject with Error instances.

Reviewed changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/plugin-cloudflare/src/tunnel.ts ??= for URL detection
packages/e2e/setup/auth.ts Remove stale eslint disable
packages/cli/src/index.ts Remove stale eslint disable
packages/cli-kit/src/public/node/ui.tsx isTTY coalescing refactor
packages/cli-kit/src/public/node/themes/factories.ts Use ?? for asset sizing
packages/cli-kit/src/public/node/themes/conf.ts Lazy init via ??=
packages/cli-kit/src/public/node/themes/api.ts Remove stale eslint disable
packages/cli-kit/src/public/node/serial-batch-processor.ts Lazy start via ??=
packages/cli-kit/src/public/node/result.ts Suppress intentional non-Error throw
packages/cli-kit/src/public/node/path.ts Use ?? for INIT_CWD
packages/cli-kit/src/public/node/os.ts Env var selection via ??
packages/cli-kit/src/public/node/mimes.ts Use ?? fallback
packages/cli-kit/src/public/node/local-storage.test.ts Remove stale eslint disables
packages/cli-kit/src/public/node/http.ts Ensure rejected values are Error
packages/cli-kit/src/public/node/hooks/prerun.ts Use ??= for command parsing
packages/cli-kit/src/public/node/global-context.ts Lazy init via ??=
packages/cli-kit/src/public/node/error-handler.ts Ensure rejected values are Error
packages/cli-kit/src/public/node/dot-env.ts Use ?? for regex capture fallback
packages/cli-kit/src/public/node/api/admin.ts ??= for API version resolution
packages/cli-kit/src/public/node/analytics.ts ?? defaults for metrics fields
packages/cli-kit/src/private/node/ui/hooks/use-select-state.ts Use ??= for first item
packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx Use ?? for link label fallback
packages/cli-kit/src/private/node/ui/components/Table/Table.tsx Use ?? for header fallbacks
packages/cli-kit/src/private/node/conf-store.ts Lazy init via ??=
packages/app/src/cli/utilities/json-schema.ts Use ?? for errors array
packages/app/src/cli/utilities/extensions/theme/host-theme-manager.ts Use ??= for lazy create
packages/app/src/cli/utilities/developer-platform-client/partners-client.ts Singleton init via ??=
packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts Singleton init via ??=
packages/app/src/cli/services/webhook/trigger-options.ts Use ?? for prompts/defaults
packages/app/src/cli/services/payments/extension-config-builder.ts Use ?? for context resolution
packages/app/src/cli/services/local-storage.ts Lazy init via ??=
packages/app/src/cli/services/init/template/npm.ts Use ?? for deps object
packages/app/src/cli/services/info.test.ts Use ?? in object spread
packages/app/src/cli/services/generate.ts Use ?? for registrationLimit
packages/app/src/cli/services/function/build.ts Use ?? for targeting default
packages/app/src/cli/services/flow/serialize-partners-fields.ts Use ?? for description
packages/app/src/cli/services/extensions/common.ts Use ?? for template path
packages/app/src/cli/services/dev/update-extension.test.ts Inline suppress reject-errors in tests
packages/app/src/cli/services/dev/select-app.ts Remove negated condition
packages/app/src/cli/services/dev/processes/uninstall-webhook.ts Use ?? fallbacks
packages/app/src/cli/services/dev/graphiql/server.ts Targeted lint suppressions + ??=
packages/app/src/cli/services/dev/extension/localization.ts Use ?? over ternary
packages/app/src/cli/services/dev.ts Use ??= for port selection
packages/app/src/cli/services/context/identifiers-extensions.ts Use ?? for URI fallback
packages/app/src/cli/services/context/breakdown-extensions.ts Use ?? for version fallback
packages/app/src/cli/services/build/steps/include-assets/copy-source-entry.ts Remove negated condition
packages/app/src/cli/services/build/steps/include-assets-step.ts Remove negated condition
packages/app/src/cli/services/app/select-app.ts Use ?? for version/modules defaults
packages/app/src/cli/services/app-logs/utils.ts Prefer ?? over `
packages/app/src/cli/services/app-logs/logs-command/ui/components/Logs.tsx Use ?? for render fallback
packages/app/src/cli/services/app-logs/logs-command/render-json-logs.ts Use ?? for cursor
packages/app/src/cli/services/app-logs/logs-command/poll-app-logs.test.ts Use ?? defaults in test helper
packages/app/src/cli/services/app-logs/dev/poll-app-logs.ts Use ?? for cursor
packages/app/src/cli/services/app-context.ts Use ??= for remoteApp fetch
packages/app/src/cli/prompts/init/init.ts Use ??= and ?? fallbacks
packages/app/src/cli/prompts/generate/extension.ts Use ?? for name fallback
packages/app/src/cli/models/app/identifiers.ts Use ??= for dotenv init
packages/app/src/cli/commands/app/webhook/trigger.ts Use ?? for secret fallback
packages/app/src/cli/commands/app/function/run.ts Use ?? for export fallback
packages/app/src/cli/commands/app/app-logs/sources.ts Remove negated condition

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@isaacroldan isaacroldan force-pushed the 03-26-chore_fix_87_eslint_warnings_across_the_codebase branch from 07d02b4 to 5965a70 Compare March 26, 2026 15:18
@isaacroldan isaacroldan force-pushed the 03-26-chore_fix_87_eslint_warnings_across_the_codebase branch 3 times, most recently from d167d8f to 731c405 Compare March 27, 2026 09:18
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 82.28% 15095/18345
🟡 Branches 74.85% 7430/9927
🟢 Functions 81.29% 3794/4667
🟢 Lines 82.67% 14273/17264

Test suite run success

3993 tests passing in 1527 suites.

Report generated by 🧪jest coverage report action from ccb06f4

@isaacroldan isaacroldan force-pushed the 03-26-chore_fix_87_eslint_warnings_across_the_codebase branch 2 times, most recently from 7d8f828 to b23df55 Compare March 27, 2026 10:23
Resolve the majority of eslint warnings (107 → 20 remaining):

- Replace 72 || with ?? (prefer-nullish-coalescing) for safer null/undefined handling
- Replace 18 if-assign patterns with ??= for conciseness
- Replace 4 ternaries with ?? where equivalent
- Remove 4 stale eslint-disable directives
- Fix 2 prefer-promise-reject-errors by wrapping in Error
- Fix 4 no-negated-condition by flipping if/else branches
- Suppress 1 only-throw-error in Result pattern (intentional generic TError)
- Suppress 5 prefer-promise-reject-errors in tests (intentional non-Error rejections)
- Suppress 1 prefer-nullish-coalescing where empty string fallthrough is intended

The 20 remaining warnings are all switch-exhaustiveness-check which require
domain-specific decisions about handling new enum/union variants.

Co-authored-by: Claude Code <claude-code@anthropic.com>
@isaacroldan isaacroldan force-pushed the 03-26-chore_fix_87_eslint_warnings_across_the_codebase branch from b23df55 to ccb06f4 Compare March 27, 2026 10:25
Copy link
Copy Markdown
Contributor

@alfonso-noriega alfonso-noriega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great clean up!

@isaacroldan isaacroldan added this pull request to the merge queue Mar 27, 2026
Merged via the queue into main with commit 2591d56 Mar 27, 2026
29 checks passed
@isaacroldan isaacroldan deleted the 03-26-chore_fix_87_eslint_warnings_across_the_codebase branch March 27, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants