-
Notifications
You must be signed in to change notification settings - Fork 142
Simple utils ts migration #847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR attempts an initial TypeScript migration by converting a small util (checksum) to TS, but also expands into a broader tooling/build migration (pnpm, new tsconfigs, ESLint v9 flat config, workflow/script updates) and wide repo-wide formatting changes.
Changes:
- Add TS build/typecheck scaffolding (
tsconfig*.json) and Vitest configuration + a new TS unit test forverifyChecksum. - Migrate
src/util/checksumfrom JS to TS with explicit parameter/return types. - Switch tooling/CI/scripts toward pnpm + ESLint v9 flat config, and reformat many files to match updated Prettier settings (double quotes).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.mts | Adds Vitest configuration for running new TS tests. |
| types/test.ts | Updates type expectations for newer Node Buffer generic typing. |
| types/.eslintrc.js | Formatting change (quote style). |
| type_validation/util/checksum.d.ts | Adds generated/declared TS typings for migrated checksum util. |
| tsconfig.json | Adds root TS config for IDE/typecheck (noEmit). |
| tsconfig.cjs.json | Adds TS config intended to compile CJS output. |
| tsconfig.base.json | Introduces shared strict TS compiler options baseline. |
| test/unit/util/continued_fraction_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/util/checksum.test.ts | Adds Vitest coverage for verifyChecksum behavior. |
| test/unit/util/bignumber_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/transaction_envelope_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/sorobandata_builder_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/soroban_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/signing_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/signerkey_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/scint_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/operations/invoke_host_function_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/operations/extend_restore_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/muxed_account_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/memo_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/liquidity_pool_id_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/keypair_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/invocation_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/i256_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/hashing_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/events_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/crypto_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/contract_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/claimant_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/auth_test.js | Formatting changes to align with updated Prettier settings. |
| test/unit/account_test.js | Formatting changes to align with updated Prettier settings. |
| test/test-helper.js | Switches Node test bootstrap from Babel register to ts-node register. |
| test/test-helper-browser.js | Extracts browser-only chai-as-promised setup for Karma runs. |
| test/.eslintrc.js | Formatting change (quote style). |
| src/xdr.js | Formatting change (quote style) for imports. |
| src/util/util.js | Formatting change (quote style). |
| src/util/decode_encode_muxed_account.js | Formatting change (quote style) + string literal normalization. |
| src/util/continued_fraction.js | Formatting change (quote style) + removes now-unneeded eslint-disable comments. |
| src/util/checksum.ts | Migrates checksum util to TS with explicit types. |
| src/util/bignumber.js | Formatting change (quote style). |
| src/transaction_builder.js | Formatting change (quote style) + string literal normalization. |
| src/transaction_base.js | Formatting change (quote style) + string literal normalization. |
| src/transaction.js | Formatting change (quote style) + string literal normalization. |
| src/sorobandata_builder.js | Formatting change (quote style) + string literal normalization. |
| src/soroban.js | Formatting change (quote style) + string literal normalization. |
| src/signing.js | Formatting change (quote style). |
| src/signerkey.js | Formatting change (quote style) + string literal normalization. |
| src/scval.js | Formatting change (quote style) + string literal normalization. |
| src/operations/set_trustline_flags.js | Formatting change (quote style) + string literal normalization. |
| src/operations/set_options.js | Formatting change (quote style) + removes/relocates eslint directives. |
| src/operations/revoke_sponsorship.js | Formatting change (quote style) + string literal normalization. |
| src/operations/restore_footprint.js | Formatting change (quote style). |
| src/operations/payment.js | Formatting change (quote style) + string literal normalization. |
| src/operations/path_payment_strict_send.js | Formatting change (quote style) + string literal normalization. |
| src/operations/path_payment_strict_receive.js | Formatting change (quote style) + string literal normalization. |
| src/operations/manage_sell_offer.js | Formatting change (quote style) + string literal normalization. |
| src/operations/manage_data.js | Formatting change (quote style) + string literal normalization. |
| src/operations/manage_buy_offer.js | Formatting change (quote style) + string literal normalization. |
| src/operations/liquidity_pool_withdraw.js | Formatting change (quote style) + string literal normalization. |
| src/operations/liquidity_pool_deposit.js | Formatting change (quote style) + string literal normalization. |
| src/operations/invoke_host_function.js | Formatting change (quote style) + string literal normalization. |
| src/operations/inflation.js | Formatting change (quote style). |
| src/operations/index.js | Formatting change (quote style) for exports. |
| src/operations/extend_footprint_ttl.js | Formatting change (quote style) + string literal normalization. |
| src/operations/end_sponsoring_future_reserves.js | Formatting change (quote style). |
| src/operations/create_passive_sell_offer.js | Formatting change (quote style) + string literal normalization. |
| src/operations/create_claimable_balance.js | Formatting change (quote style) + string literal normalization. |
| src/operations/create_account.js | Formatting change (quote style) + string literal normalization. |
| src/operations/clawback_claimable_balance.js | Formatting change (quote style) + string literal normalization. |
| src/operations/clawback.js | Formatting change (quote style) + string literal normalization. |
| src/operations/claim_claimable_balance.js | Formatting change (quote style) + string literal normalization. |
| src/operations/change_trust.js | Formatting change (quote style) + string literal normalization. |
| src/operations/bump_sequence.js | Formatting change (quote style) + string literal normalization. |
| src/operations/begin_sponsoring_future_reserves.js | Formatting change (quote style) + string literal normalization. |
| src/operations/allow_trust.js | Formatting change (quote style) + string literal normalization. |
| src/operations/account_merge.js | Formatting change (quote style) + string literal normalization. |
| src/numbers/xdr_large_int.js | Formatting change (quote style) + string literal normalization. |
| src/numbers/uint256.js | Formatting change (quote style). |
| src/numbers/uint128.js | Formatting change (quote style). |
| src/numbers/sc_int.js | Formatting change (quote style) + string literal normalization. |
| src/numbers/int256.js | Formatting change (quote style). |
| src/numbers/int128.js | Formatting change (quote style). |
| src/numbers/index.js | Formatting change (quote style) + string literal normalization. |
| src/network.js | Formatting change (quote style). |
| src/muxed_account.js | Formatting change (quote style) + string literal normalization. |
| src/memo.js | Formatting change (quote style) + string literal normalization. |
| src/liquidity_pool_id.js | Formatting change (quote style) + string literal normalization. |
| src/liquidity_pool_asset.js | Formatting change (quote style) + string literal normalization. |
| src/keypair.js | Formatting change (quote style) + string literal normalization. |
| src/jsxdr.js | Formatting change (quote style). |
| src/invocation.js | Formatting change (quote style) + string literal normalization. |
| src/index.js | Formatting change (quote style) + string literal normalization. |
| src/hashing.js | Formatting change (quote style). |
| src/get_liquidity_pool_id.js | Formatting change (quote style) + string literal normalization. |
| src/fee_bump_transaction.js | Formatting change (quote style) + string literal normalization. |
| src/events.js | Formatting change (quote style). |
| src/contract.js | Formatting change (quote style). |
| src/claimant.js | Formatting change (quote style) + string literal normalization. |
| src/auth.js | Formatting change (quote style) + string literal normalization. |
| src/asset.js | Formatting change (quote style) + string literal normalization. |
| src/address.js | Formatting change (quote style) + string literal normalization. |
| src/account.js | Formatting change (quote style) + string literal normalization. |
| package.json | Switches package entrypoints/build/test/lint scripts toward pnpm + tsc CJS build + adds Vitest scripts. |
| mocharc.tsnode.json | Moves Mocha config out of package.json into a dedicated file. |
| examples/signing-data.js | Formatting change (quote style). |
| config/webpack.config.browser.js | Updates browser build pipeline to optionally support TS entrypoints and TS transpilation. |
| config/prettier.config.cjs | Updates Prettier settings (notably singleQuote: false). |
| config/karma.conf.js | Updates Karma config for pnpm + explicit plugin requires + new browser test helper. |
| config/eslint.config.cjs | Introduces ESLint v9 flat config (JS + TS). |
| config/.eslintrc.js | Removes legacy ESLint config in favor of flat config. |
| README.md | Updates docs to reference pnpm and reflows warning sections. |
| CONTRIBUTING.md | Updates contributor docs to reference pnpm and reflows content. |
| .github/workflows/tests.yml | Updates CI to install/use pnpm and run pnpm scripts. |
| .github/workflows/npm_publish.yml | Updates publish workflow to install/use pnpm and run pnpm scripts. |
| .github/workflows/gh_pages.yml | Updates GH Pages workflow to install/use pnpm and run pnpm scripts. |
| .github/workflows/format.yml | Updates formatting workflow to install/use pnpm and run pnpm scripts. |
| .github/workflows/bundle_size.yml | Updates bundle size workflow to install/use pnpm and run pnpm scripts. |
| .github/ISSUE_TEMPLATE/bug_report.md | Updates issue template to reference pnpm lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I believe the workflow checks are failing because I initially had this PR merge into master and it still thinks that after fixing and rerunning |
quietbits
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good start! Let's close this PR and open a new one to make sure all the checks pass. It would be helpful to add comments about config changes and any steps we would need to take and why.
| @@ -1,5 +1,3 @@ | |||
| /* eslint-disable no-param-reassign */ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we removing these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous eslint config used a stricter rule set
| @@ -0,0 +1 @@ | |||
| export declare function verifyChecksum(expected: Uint8Array, actual: Uint8Array): boolean; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to create these manually for every file we migrate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generated by tsc
|
|
||
| // JavaScript support | ||
| "allowJs": true, | ||
| "allowJs": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll have mixed JS and TS files for a while, is it OK to disable it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose to disable this so that it would be easier to review the generation type declaration files. With this flag set to true the tsc compiler will generate declaration files for all js files.
Converts a single file in the util folder to check migration process