Skip to content

INTER-2281: upgrade dx-team-toolkit dev dependencies to latest#249

Open
JuroUhlar wants to merge 16 commits into
mainfrom
claude/upgrade-dx-toolkit-deps-q51eef
Open

INTER-2281: upgrade dx-team-toolkit dev dependencies to latest#249
JuroUhlar wants to merge 16 commits into
mainfrom
claude/upgrade-dx-toolkit-deps-q51eef

Conversation

@JuroUhlar

@JuroUhlar JuroUhlar commented Jul 1, 2026

Copy link
Copy Markdown
Contributor
  • Upgrades dx-team lint/changelog/prettier tooling and migrates ESLint to flat type-checked config.
  • Normalizes caught values into real Error causes; includes a patch changeset.
  • Adds tooling typecheck coverage and documents Node 22 for local development.

Note

eslint is provided by the dx-team ESLint config; prettier stays direct because it is a peer dependency.

Verified with pnpm lint, pnpm test, pnpm build, and typecheck scripts.

@changeset-bot

changeset-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 86c5349

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 100% 357/357
🟢 Branches 100% 124/124
🟢 Functions 100% 34/34
🟢 Lines 100% 357/357

Test suite run success

79 tests passing in 24 suites.

Report generated by 🧪jest coverage report action from 86c5349

Show full coverage report
St File % Stmts % Branch % Funcs % Lines Uncovered Line #s
🟢 All files 100 100 100 100
🟢  src 100 100 100 100
🔴   ...edApiTypes.ts 0 0 0 0
🟢   index.ts 100 100 100 100
🟢   sealedResults.ts 100 100 100 100
🟢   ...rApiClient.ts 100 100 100 100
🟢   types.ts 100 100 100 100
🟢   urlUtils.ts 100 100 100 100
🟢   webhook.ts 100 100 100 100
🟢  src/errors 100 100 100 100
🟢   apiErrors.ts 100 100 100 100
🟢   ...orResponse.ts 100 100 100 100
🟢   toError.ts 100 100 100 100
🟢   unsealError.ts 100 100 100 100

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Upgrades the repo’s DX team toolkit dev dependencies and migrates ESLint from legacy .eslintrc to ESLint 10 flat config, with small source edits to satisfy the stricter lint rules.

Changes:

  • Upgraded @fingerprintjs/*-dx-team tooling packages and added explicit eslint@10 + prettier@3.
  • Migrated ESLint configuration to eslint.config.js and adjusted the lint script accordingly.
  • Applied minor code tweaks / lint suppressions (nullish checks, assertion-related rule overrides, formatting).

Reviewed changes

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

Show a summary per file
File Description
src/urlUtils.ts Replaces == null with explicit `=== null
src/serverApiClient.ts Adds documented eslint suppressions for type assertions used in option normalization and JSON parsing.
src/sealedResults.ts Adds eslint suppression + commentary around caught-error assertion in unsealing flow.
src/errors/toError.ts Adds eslint suppression + commentary around returning non-Error objects as Error.
src/errors/handleErrorResponse.ts Formatting/indentation adjustment in the error-response type guard.
rollup.config.js Suppresses no-require-imports for require('./package.json').
package.json Updates lint script; bumps DX-team tooling deps; adds explicit eslint and prettier.
eslint.config.js Introduces ESLint flat config with ignores and test-file rule relaxations.
.eslintrc.json Removes legacy ESLint config file.
pnpm-lock.yaml Lockfile updates reflecting new toolchain versions and dependency graph.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

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

Comment thread src/errors/toError.ts Outdated
Comment thread src/sealedResults.ts
Comment thread package.json Outdated
@JuroUhlar JuroUhlar changed the base branch from main to claude/jest-vitest-migration-a3158t July 2, 2026 20:43
claude and others added 6 commits July 2, 2026 21:48
Upgrade the shared FingerprintJS DX team toolkit dev dependencies:

- @fingerprintjs/eslint-config-dx-team: 0.1.0 -> 2.0.0
- @fingerprintjs/conventional-changelog-dx-team: 0.1.0 -> 0.2.0
- @fingerprintjs/prettier-config-dx-team: 0.2.0 -> 0.3.0

The new eslint config is a flat config built on ESLint 10 and
typescript-eslint 8, so migrate accordingly:

- Replace .eslintrc.json with eslint.config.js (flat config)
- Add explicit eslint and prettier dev dependencies
- Drop the removed --ext / --ignore-path flags from the lint script
- Relax consistent-type-assertions / no-explicit-any for test files
  (mocking legitimately relies on assertions such as
  `fetch as unknown as jest.Mock`)

Fix issues surfaced by the stricter ruleset:

- Use strict equality for nullish checks in urlUtils
- Add explained eslint-disable directives for the few unavoidable type
  assertions in source (assertionStyle: never) and the require() import
  in rollup.config.js
- Apply prettier 3.9 formatting

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRhyyBoYDJH5Aoq8MqXCNF
Address Copilot review feedback on the type-assertion suppressions:

- toError: return the value directly when it is already an Error, and
  otherwise construct a real Error (preserving the message for error-like
  objects that are not Error instances, e.g. cross-realm values from
  native bindings). Removes the misleading "safe" assertion.
- sealedResults: normalize the caught value via toError instead of
  asserting `e as Error`, so UnsealError always receives a real Error.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RRhyyBoYDJH5Aoq8MqXCNF
@JuroUhlar JuroUhlar force-pushed the claude/upgrade-dx-toolkit-deps-q51eef branch from 719a58c to bfc8d9f Compare July 3, 2026 10:18
@github-actions

This comment has been minimized.

JuroUhlar added 2 commits July 3, 2026 12:51
Switch to @fingerprintjs/eslint-config-dx-team/type-checked and remove
eslint from devDependencies since the preset bundles it.
Adjust type guards and sealedResults switch, extend test/example
eslint overrides, and restore region cast in serverApiClient.
@JuroUhlar JuroUhlar force-pushed the claude/upgrade-dx-toolkit-deps-q51eef branch from bfc8d9f to 23e6a76 Compare July 3, 2026 11:51
JuroUhlar added 3 commits July 3, 2026 14:10
Add tooling tsconfig for vitest/generate, trim test rule overrides,
fix lint issues in tests and generate.mts, and switch eslint config to ESM.
Point ESLint at tests/tsconfig.json so Vitest types resolve in the IDE.
@JuroUhlar JuroUhlar requested a review from Copilot July 3, 2026 13:53
@JuroUhlar JuroUhlar changed the title chore(deps): upgrade dx-team-toolkit dev dependencies to latest INTER-2281: upgrade dx-team-toolkit dev dependencies to latest Jul 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread rollup.config.js
Comment thread src/sealedResults.ts
Comment thread contributing.md Outdated
@github-actions

This comment has been minimized.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@JuroUhlar JuroUhlar force-pushed the claude/upgrade-dx-toolkit-deps-q51eef branch from 6a3b1bb to 2bfde62 Compare July 3, 2026 14:01
@JuroUhlar JuroUhlar marked this pull request as ready for review July 3, 2026 14:06
Base automatically changed from claude/jest-vitest-migration-a3158t to main July 6, 2026 13:50
Orkuncakilkaya
Orkuncakilkaya previously approved these changes Jul 6, 2026
Comment thread src/serverApiClient.ts Outdated
this.region = (options.region as Region) ?? Region.Global
// Options.region accepts string literals for the same runtime values as the enum.
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- string literal union is not assignable to Region
this.region = (options.region ?? Region.Global) as Region

@Orkuncakilkaya Orkuncakilkaya Jul 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: We may still use @typescript-eslint/consistent-type-assertions here by using const instead of enum.

// src/types.ts

export const Region = {
  EU: 'EU',
  AP: 'AP',
  Global: 'Global',
} as const

export type Region = (typeof Region)[keyof typeof Region]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, thanks! Done in 985291f

Lets consistent-type-assertions stay enabled without a disable in
serverApiClient, since options.region is now assignable to Region
directly. Runtime output is identical to the string enum.
The const object union narrows `let region = Region.Global` to the
"Global" literal, so reassigning Region.EU/AP failed typecheck:examples.
JSDoc @type {Region} widens it back.
@JuroUhlar JuroUhlar requested a review from Orkuncakilkaya July 7, 2026 14:54
mcnulty-fp
mcnulty-fp previously approved these changes Jul 7, 2026
Comment thread src/errors/toError.ts Outdated
return new Error(String(error.message))
}

return new Error(String(error))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the error object doesn't have a toString function, String(error) will result in [object Object]. Since message is checked as being a property on error, perhaps this should use message somehow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure I follow, if message exists the conditional code block above returns the error using error.message. This line (13) only executes if message does not exist, no?

Or are you proposing something like this?

Suggested change
return new Error(String(error))
return new Error(JSON.stringify(error))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure I follow, if message exists the conditional code block above returns the error using error.message. This line (13) only executes if message does not exist, no?

Sorry! I misread the diff because the "Branch is not covered" annotations made it difficult to read. I've found the "Show annotations" setting and disabled it now 🤦 .

Or are you proposing something like this?

I think we should probably only use the result of String(error) if it'll provide a meaningful message. So, maybe the check can be something like?

const message = String(error)
if (message !== '[object Object]') {
   return new Error(message)
}

return new Error(JSON.stringify(error))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like that, thank you! Also added basic tests to cover this case: 86c5349

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🚀 Following releases will be created using changesets from this PR:

@fingerprint/node-sdk@7.4.1

Patch Changes

  • errors: Normalize caught values into real Error instances, so errors from unseal and JSON parsing always carry a genuine Error. (5764c11)

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.

5 participants