Skip to content

Consume @itwin/eslint-plugin 6.0.0, update linting, remove a pnpm override#342

Merged
hl662 merged 7 commits intomainfrom
nam/eslint-bump
Mar 27, 2026
Merged

Consume @itwin/eslint-plugin 6.0.0, update linting, remove a pnpm override#342
hl662 merged 7 commits intomainfrom
nam/eslint-bump

Conversation

@hl662
Copy link
Copy Markdown
Contributor

@hl662 hl662 commented Mar 20, 2026

Follow up from #341 (comment)

Dependency upgrades:

  • Upgraded @itwin/eslint-plugin to version 6.0.0 and eslint to version 9.11.1 in packages/browser/package.json and packages/electron/package.json, ensuring all packages use the latest linting tools.
  • Remove the pnpm override for flatten, now that we don't need it

Lint rule adjustments and code style:

  • Replaced deprecated lint rule comments (deprecation/deprecation and @typescript-eslint/no-var-requires) with updated ones (@typescript-eslint/no-deprecated and @typescript-eslint/no-require-imports) throughout the codebase for improved lint compatibility.
  • Updated deprecated documentation tags to specify version numbers more clearly (e.g., @deprecated in 1.1.0.). We also have a custom lint rule in iTwin/eslint-plugin that is more strict on this
  • Improved error handling style by removing unused error variables in catch blocks, leading to cleaner and more modern code.
  • Added explicit lint disables for non-null assertions to satisfy new linting requirements.

Other minor improvements:

  • Removed an unnecessary override for flat-cache>flatted in the root package.json.
  • Minor formatting and naming convention fixes for improved readability and consistency.

These changes collectively modernize the codebase’s linting setup and ensure compatibility with the latest standards.

hl662 and others added 3 commits March 20, 2026 14:39
- Bump @itwin/eslint-plugin ^4.1.1 → ^6.0.0 and eslint ^8.57.1 → ^9.11.1 in all 5 packages
- Remove flat-cache>flatted pnpm override (no longer needed: eslint@9 brings flatted@3.4.2)
- Fix all lint errors introduced by v6 rule changes:
  - Replace deprecated 'deprecation/deprecation' disable comments with '@typescript-eslint/no-deprecated'
  - Update '@typescript-eslint/no-var-requires' disable comments to 'no-require-imports'
  - Replace unused catch bindings (err/error) with bare catch {} blocks
  - Add eslint-disable-next-line for no-non-null-assertion sites (per project policy)
  - Fix prefer-promise-reject-errors violations
  - Fix @itwin/require-version-in-deprecation: use '. ' separator, no 'x' in semver
  - Remove stale/duplicate eslint-disable comments made unnecessary by v6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@anmolshres98 anmolshres98 left a comment

Choose a reason for hiding this comment

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

are all the eslint disable lines needed? could null assertions be correctly checked instead of disabling it for specific lines? perhaps you can quickly evaluate?

…n-null assertions

- Remove dormant minimatch pnpm override (no package uses minimatch 9.x)
- Convert require() to top-level imports in TestConfig.ts, add dotenv/dotenv-expand as devDeps
- Replace non-null assertions with proper null guards in production code (Client.ts, IntrospectionClient.ts) and integration test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hl662
Copy link
Copy Markdown
Contributor Author

hl662 commented Mar 27, 2026

Re: review comment on eslint disables and non-null assertions —

Went through all the no-non-null-assertion disables in the repo. Replaced the ones in production code and the integration test with proper null guards:

  • Client.ts (2 spots): _configuration and authRequest.internal — added defensive checks with descriptive errors
  • IntrospectionClient.ts: Map.get() result — added a guard instead of asserting
  • integration.test.ts: process.env.APPDATA — added a guard

Left the test-only ones (MainAuthorizationClient.test.ts, TokenStore.test.ts, Introspection.test.ts) as-is since they are low risk.

The @typescript-eslint/no-deprecated disables are intentional — those test deprecated APIs to ensure they still work correctly.

hl662 and others added 2 commits March 27, 2026 11:37
Override picomatch@<2.3.2 and picomatch@>=4.0.0<4.0.4 to patched
versions, resolving ReDoS vulnerability (GHSA-c2c7-rcm5-vvqj) from
transitive deps via mocha/chokidar, beachball, and typescript-eslint.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Bump mocha from ^10.8.2 to ^11.7.5 across all packages. mocha 11 uses
chokidar@4 which no longer depends on the vulnerable picomatch@4.0.3,
removing the need for the picomatch@>=4.0.0<4.0.4 pnpm override.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@anmolshres98 anmolshres98 left a comment

Choose a reason for hiding this comment

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

I think you committed this one file by mistake other than that looks good I think. thanks

- certaBackend.ts: convert dotenv/dotenv-expand requires to imports
- loadConfig.ts: convert dotenv-expand require to import
- Upgrade dotenv-expand to v12 in oidc-signin-tool
- TokenStore.ts and renderer/Client.ts: cannot convert due to
  dual CJS/ESM build and runtime conditional loading respectively

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hl662 hl662 force-pushed the nam/eslint-bump branch from 289c06e to 4d6abba Compare March 27, 2026 18:45
@hl662 hl662 enabled auto-merge (squash) March 27, 2026 18:48
@hl662 hl662 merged commit 517ef25 into main Mar 27, 2026
13 checks passed
@hl662 hl662 deleted the nam/eslint-bump branch March 27, 2026 19:12
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.

3 participants