chore: upgrade to TS 5.8 #17537
chore: upgrade to TS 5.8 #17537
Conversation
Nodonisko
requested review from
szymonlesisz,
a team,
mroz22,
martykan and
marekrjpolak
as code owners
Nodonisko
added
monorepo
WalkthroughThis pull request includes updates across several project files. Multiple package.json files have been revised to update dependency versions—most notably for TypeScript, @types/node, and long—across various packages and examples. In addition, the mobile expo project now uses a fixed version of TypeScript instead of a range. Changes in the source code adjust the handling of Buffer instances, ensuring that values are correctly cast or converted to hexadecimal strings in specific utility functions. The method signature in a hashing function has been broadened to accept a wider range of buffer types. Furthermore, the JSX formatter component was reformatted to use an explicit return syntax, and the compiler target in the TypeScript configuration has been updated from "esnext" to "ES2023". These modifications standardize dependency versions and improve type handling throughout the codebase. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (20)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
suite-common/formatters/src/makeFormatter.tsx (1)
47-49: Improved syntax for JSX component returnThe component has been updated to use explicit parentheses for the JSX return value, which is a good practice for clarity. However, I noticed that the null coalescing operator (
?? null) has been removed, which means the component will now return an empty fragment instead of null when the formatted output is nullish.While this doesn't affect typical usage patterns, it's worth noting that the component interface on line 33 allows for returning
JSX.Element | null. The updated implementation aligns better with React best practices but has a slightly different behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
package.json(3 hunks)packages/connect-examples/mobile-expo/package.json(1 hunks)packages/connect/src/api/cipherKeyValue.ts(1 hunks)packages/protobuf/package.json(1 hunks)packages/protobuf/tsconfig.json(1 hunks)packages/trezor-user-env-link/package.json(1 hunks)packages/utxo-lib/src/transaction/zcash.ts(1 hunks)suite-common/formatters/src/makeFormatter.tsx(1 hunks)suite-native/app/package.json(2 hunks)tsconfig.base.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: build-web
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: transport-e2e-test
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: test
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (12)
packages/protobuf/package.json (1)
37-39: Upgrade Dependency "long" Version
Thelongdependency has been updated from"5.2.0"to"5.2.5", aligning with dependency standardization across the project. Please verify that this patch version is fully backward compatible with existing usages.packages/trezor-user-env-link/package.json (1)
17-19: Update TypeScript Version in DevDependencies
The TypeScript package indevDependenciesis now fixed to version"5.8.2"(up from"5.5.4"). This change helps ensure consistency across workspaces. Double-check that the updates in TS configuration (and any API changes between releases) are compatible with this version.tsconfig.base.json (1)
8-10: Change Compilation Target to ES2023
The target incompilerOptionshas been updated from"esnext"to"ES2023". This explicit target standardizes the ECMAScript features used in the output. Please ensure that all deployment environments support ES2023 features and that this change does not inadvertently impact any cross-environment compatibility.packages/connect-examples/mobile-expo/package.json (1)
18-23: Pin TypeScript Version for Mobile Expo Example
The TypeScript version underdevDependencieshas been changed from a caret version (^5.3.3) to a fixed version ("5.8.2"). This ensures consistency with other packages in the repository. Verify that Expo or any other tooling used in this package does not require a looser version range.package.json (2)
90-100: Update Resolutions for TypeScript and @types/node
Within theresolutionssection, the TypeScript version is now set to"5.8.2"and@types/nodehas been updated from"22.10.1"to"22.13.10". This change supports uniform dependency management across the project. It is advisable to verify that all packages depending on these resolutions behave as expected.
120-145: Upgrade DevDependencies for TypeScript and Node Types
In thedevDependenciessection, the TypeScript package has been updated to"5.8.2"and@types/nodeto"22.13.10". Ensure that these upgrades align with your build, lint, and test configurations and that there are no breaking changes introduced by the newer TypeScript version.packages/protobuf/tsconfig.json (1)
3-6:✅ Verification successful
Added
allowSyntheticDefaultImportsfor better module compatibility.This configuration change enables synthetic default imports, which is a good practice when working with modules that don't have explicit default exports (common in many libraries).
🏁 Script executed:
#!/bin/bash # Check if any imports in the codebase might benefit from this configuration grep -r "import \* as " packages/protobuf/src/ | head -5Length of output: 283
Approval Confirmed:
allowSyntheticDefaultImportscorrectly enables better module compatibility.The configuration update in
packages/protobuf/tsconfig.jsonis validated by source files importing modules with theimport * assyntax (e.g., inpackages/protobuf/src/index.ts,packages/protobuf/src/types.ts, andpackages/protobuf/src/utils.ts). This change is appropriate and beneficial for handling modules without explicit default exports.suite-native/app/package.json (2)
139-139: Updated Node types version to match TypeScript upgrade.The update to @types/node v22.13.10 is appropriate for compatibility with the TypeScript upgrade.
148-148: Pinned TypeScript version for consistency.Good practice to use a fixed version (5.8.2) rather than caret range for TypeScript, ensuring all developers use exactly the same compiler version.
packages/utxo-lib/src/transaction/zcash.ts (1)
312-315: Improved Buffer handling for TypeScript 5.8 compatibility.This change properly handles Buffer types with the updated TypeScript version. The refactoring:
- Uses a more specific Buffer type signature for better type safety
- Creates a variable to hold the converted buffer, improving code readability
- Maintains identical functionality while adapting to TypeScript's stricter type system
packages/connect/src/api/cipherKeyValue.ts (2)
37-40: Enhanced Buffer handling for TypeScript 5.8 compatibility.The refactoring improves type safety by explicitly casting non-Buffer values to string type while maintaining the same functionality. The multi-line formatting also improves readability.
44-44: Explicit type casting for IV parameter.Similar to the value parameter change, this improvement adds explicit type casting for better compatibility with TypeScript 5.8's stricter type checking.
Nodonisko
force-pushed
the
feat/upgrade-ts
branch
from
4594163 to
4a0a83a
Compare
|
🚀 Expo preview is ready!
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring: Next stepsTake a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with |
Looks like a valid point, @Nodonisko. |
|
@SocketSecurity ignore npm/@eslint/[email protected] |
Description
I had to set target to ES2023 because 24 introduces some changes to Buffer typings which caused huge amount of error across mostly codebase. I don't see it as issue to have it set for ES2023.
There was few minor changes necessary but it should not break anything.
Also updated ESlint because it's kind of dependant on TS version.
Related Issue
Resolve
Screenshots: