chore: js tooling improvements #609

fvictorio · 2024-08-21T07:22:07Z

This PR started as an attempt to improve some minor things on our js-related tools, but it became something a bit bigger. I recommend reviewing it commit by commit, but I'll explain the changes here too.

Syncpack

I added a new tool, syncpack, that works as a linter for our package.json. The main things it does are:

Make sure that all dependencies used by multiples packages have the same version. You can think of this as a workaround to the fact that pnpm doesn't have what cargo calls workspace dependencies.
Formats the package.jsons, including things like sorting the keys. I don't care much about this, but it can help to reduce merge conflicts and in any case it won't hurt.
Enforces certain ranges for some dependencies. I didn't use this a lot yet, except to enforce that the typescript dependency has to have a ~ range instead of a ^ (because typescript doesn't follow semver and can introduce breaking changes in minor versions).

setup-node and setup-rust actions

I added a setup-node action so that bumping the node version requires less changes. I also added a setup-rust action; this one is not as necessary, but it does reduce the amount of boilerplate in the workflows.

In both cases, there are some places where these custom actions are not used. For example, the setup-rust action is not used when the nightly toolchain needs to be installed. And the setup-node is not used in one of the edr-npm-release jobs because that one always installs pnpm but it conditionally installs node.

Migrate tools/js/benchmark to typescript

I might regret this because it will generate some merge conflicts with the solidity tests branch, but I guess we'd have to do it sooner or later.

Better linting in general

The main thing here is that running pnpm run lint at the root of the project should do most of the linting: it will run syncpack, prettier, and then recursively run the lint script in each package. As part of this I also made sure that all our packages are using eslint.

changeset-bot · 2024-08-21T07:22:11Z

⚠️ No Changeset found

Latest commit: 46d0eb3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Wodann

Thanks; very helpful tooling!

Wodann · 2024-08-21T14:43:43Z

.github/workflows/hardhat-tests.yml

@@ -55,9 +42,6 @@ jobs:
            hardhat-tests/test/internal/hardhat-network/stack-traces/test-files/artifacts
          key: hardhat-network-stack-traces-tests-${{ hashFiles('hardhat-tests/test/internal/hardhat-network/stack-traces/test-files/**/*.sol') }}-${{ hashFiles('hardhat-tests/test/internal/hardhat-network/stack-traces/test-files/**/test.json') }}-${{ hashFiles('hardhat-tests/test/internal/hardhat-network/stack-traces/**/*.ts') }}

-      - name: Build


Just validating my understanding: this is not necessary because the test command has a pre-test step that builds.

Yes, exactly. That wasn't the case before (because the pretest script was only running build:edr), but now both EDR is built and the tests code is type-checked before running the tests.

agostbiro · 2024-08-21T14:49:38Z

crates/tools/js/benchmark/package.json

+  "license": "ISC",
+  "private": true,
+  "scripts": {
+    "benchmark": "node --noconcurrent_sweeping --noconcurrent_recompilation --max-old-space-size=28000 --import tsx src/index.ts benchmark",


This is failing for me when running pnpm benchmark with

> node --noconcurrent_sweeping --noconcurrent_recompilation --max-old-space-size=28000 --import tsx src/index.ts benchmark node:internal/errors:496 ErrorCaptureStackTrace(err); ^ TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts" for /.../edr/crates/tools/js/benchmark/src/index.ts

The weird thing is that the CI somehow passed in spite of this: https://github.com/NomicFoundation/edr/actions/runs/10489108004/job/29052994490

Are you using node v20?

Ah thanks, I thought I did, but I didn't. Works now with node 20 🙏

* fix: use remote chain id for pre-fork simulation (#567) * fix: use remote chain id for pre-fork simulation * Fix error message * chore: Bump hardhat to 2.22.7 (#571) * chore: Bump hardhat to 2.22.7 * fixup: Don't enable RIP-7212 in our tests * Adapt for NomicFoundation/hardhat#5411 * fix: prevent crash when returning large JSON responses (#569) * ci: update collaborator check in the benchmarks workflow (#574) * edr-0.5.1 (#559) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: add json alias property in provider response (#582) * fix: add json alias property in provider response * Create empty-bobcats-refuse.md * edr-0.5.2 (#583) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * breaking change: rename response.json to response.data (#584) * breaking change: rename response.json to response.data * Create sour-donkeys-draw.md * Update sour-donkeys-draw.md * fix: improve error message and option to skip unsupported transaction type in debug trace (#606) * fix: improve error message and option to skip unsupported tx type in debug trace * Address code review feedback * Handle unsupported transaction type requested * Fix test setup * ci: remove review-related slack notifications (#612) * chore: js tooling improvements (#609) * Fix pnpm warning * Add syncpakc * Add syncpack * Run syncpack format * Run syncpack on CI * Add setup-node action * Fixes related to upgrading prettier * Run prettier * Add setup-rust action * Run prettier in edr_napi * Add lint scripts to packages * Run prettier in crates/tools/js/benchmark * Port benchmark code to typescript * Add eslint to all packages * Upgrade @types/node to v20 * Fix broken build:edr script * Resolve benchmark output path * chore: upgrade hardhat and add patch (#613) * Upgrade Hardhat to v2.22.9 * Add patch for hardhat#5664 * chore: Bump to vanilla Hardhat 2.22.9 * chore: Regenerate the Hardhat patch to use the new EDR internals * chore: Bump @napi-rs/cli to fix duplicated napi typedefs See <napi-rs/napi-rs#2088> * fixup: Prettify test.ts * Fixes after testing in OZ (#625) * fix: add bounds checks * fix: check that steps is not empty before traversing it * fixup: formatting [skip ci] --------- Co-authored-by: Igor Matuszewski <xanewok@gmail.com> --------- Co-authored-by: Agost Biro <5764438+agostbiro@users.noreply.github.com> Co-authored-by: Wodann <Wodann@users.noreply.github.com> Co-authored-by: Piotr Galar <piotr.galar@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Franco Victorio <victorio.franco@gmail.com>

* refactor: Port the codebase model construction and EVM decoding from HH * chore: Add a script that regenerates the pnpm patch for Hardhat * patch(hardhat): Cherry-pick and adapt for the unreleased Rip7212 PR See NomicFoundation/hardhat@f944cd5 * patch(hardhat): Cherry-pick a stack trace cleanup PR See NomicFoundation/hardhat@5739893 * patch(hardhat): Use the rewritten compiler to model logic from EDR See NomicFoundation/hardhat@49c66b6 * Port `MessageTrace` and `VMTracer` machinery from Hardhat (#531) * Some small improvements * Port `MessageTrace` and `VMTracer` machinery from Hardhat * Move HH-specific ExitCode logic to edr_napi * Remove unused `ExitCode::STATIC_STATE_CHANGE` * patch(hardhat): Re-use MessageTrace and VmTracer from EDR now See NomicFoundation/hardhat@3aeeb56 * chore: Re-run pnpm build * Address PR feedback * Address PR feedback * fixup: Apply formatting * refactor: Port ContractsIdentifier to Rust (#562) * chore: Port the BytecodeTrie from ContractsIdentifier * Migrate some library utils * Migrate some Opcode helpers * Finish the ContractsIdentifier port * Remove unused get_library_address_positions * Simplify the ContractsIdentifier port * patch(hardhat): Re-use the ContractsIdentifier from EDR * fixup: Fix a simple test * refactor: Port the `VmTraceDecoder` from Hardhat (#568) * fix: Correctly deserialize BuildInfo with `_format` field * refactor: Port the VmTraceDecoder from Hardhat * fix(napi): Correctly encode and decode `MessageTrace` types * refactor: Remove now unneeded functions * patch(hardhat): Port the VmTraceDecoder from Hardhat * Reformat and remove unused imports * fixup: Use a more correct type for the XYZTrace::bytecode field * fixup: Adjust test to now optional `deployedContract` property * refactor: Port `ReturnData` and `StackTraceEntryType` (#589) * feat: Port solidity-stack-trace.ts * feat: Port return-data.ts * patch(hardhat): Port return-data.ts and solidity-stack-trace.ts * fixup: Reformat files * fixup: let's see how tests feel about a number instead of a const enum variant * Make the comment about Rust type alias more clear [skip ci] * fix: Fully transpile in Mocha tests to correctly inline const enums (#594) It seems transpile-only transpiles per module, however we need to perform full "compilation" in order to correctly see and inline values for const enums. We use const enums because that's how they are emitted by napi-rs and there's no option to change that. Rather than invest into supporting that or working around it, let's just fully compile the test harness once. See https://www.typescriptlang.org/tsconfig/#isolatedModules for more context. * refactor: Port ErrorInferrer and SolidityTracer from Hardhat (#593) * Start porting SolidityTracer * Start porting ErrorInferrer * WIP: Keep Reference in MessageTraces * Port 99% of the ErrorInferrer Except `ErrorInferrer.inferAfterTracing`. I discovered that the original code creates a shallow copy of the stack trace during select heuristics and returns the modified one or the *original* one if the heuristic does not hit. One could keep track of possible series of changes that would have to be subsequently applied if the heuristic hits and returns a modified the stack trace but instead, I want the code to do exactly what the original code did. Rather than wrap every entry in a shareable `Rc` (they are not modified), I decided to make them clonable in the next PR, which means we need to not keep the `ClassInstance` around to derive the Clone trait. * Port mapped-inlined-internal-functions-heuristics.ts * refactor: Prune StackTraceEntry.message to make it clonable See previous commit "Port 99% of the ErrorInferrer". * refactor: Drop unused inner funcs in the ErrorInferrer * feat: Finish porting `ErrorInferrer` * feat: Finish porting `SolidityTracer` * refactor: Remove now unused utils IntoEither/IntoOption * patch(hardhat): Port ErrorInferrer and SolidityTracer * fixup! refactor: Prune StackTraceEntry.message to make it clonable * fixup: Reformat * Document the unsafety surrounding napi-rs's References * refactor: Remove some now unused functions * Address review feedback * fix a missing period * fix alphabetical sorting * refactor: Port stack-traces/debug.ts (#596) * Port over debugging facilities from debug.ts * patch(hardhat): Port debug.ts * fix: Make the depth optional in printMessageTrace * fixup: Formatting * Merge `main` into the stack trace port feature branch (#618) * fix: use remote chain id for pre-fork simulation (#567) * fix: use remote chain id for pre-fork simulation * Fix error message * chore: Bump hardhat to 2.22.7 (#571) * chore: Bump hardhat to 2.22.7 * fixup: Don't enable RIP-7212 in our tests * Adapt for NomicFoundation/hardhat#5411 * fix: prevent crash when returning large JSON responses (#569) * ci: update collaborator check in the benchmarks workflow (#574) * edr-0.5.1 (#559) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: add json alias property in provider response (#582) * fix: add json alias property in provider response * Create empty-bobcats-refuse.md * edr-0.5.2 (#583) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * breaking change: rename response.json to response.data (#584) * breaking change: rename response.json to response.data * Create sour-donkeys-draw.md * Update sour-donkeys-draw.md * fix: improve error message and option to skip unsupported transaction type in debug trace (#606) * fix: improve error message and option to skip unsupported tx type in debug trace * Address code review feedback * Handle unsupported transaction type requested * Fix test setup * ci: remove review-related slack notifications (#612) * chore: js tooling improvements (#609) * Fix pnpm warning * Add syncpakc * Add syncpack * Run syncpack format * Run syncpack on CI * Add setup-node action * Fixes related to upgrading prettier * Run prettier * Add setup-rust action * Run prettier in edr_napi * Add lint scripts to packages * Run prettier in crates/tools/js/benchmark * Port benchmark code to typescript * Add eslint to all packages * Upgrade @types/node to v20 * Fix broken build:edr script * Resolve benchmark output path * chore: upgrade hardhat and add patch (#613) * Upgrade Hardhat to v2.22.9 * Add patch for hardhat#5664 * chore: Bump to vanilla Hardhat 2.22.9 * chore: Regenerate the Hardhat patch to use the new EDR internals * chore: Bump @napi-rs/cli to fix duplicated napi typedefs See <napi-rs/napi-rs#2088> * fixup: Prettify test.ts * Fixes after testing in OZ (#625) * fix: add bounds checks * fix: check that steps is not empty before traversing it * fixup: formatting [skip ci] --------- Co-authored-by: Igor Matuszewski <xanewok@gmail.com> --------- Co-authored-by: Agost Biro <5764438+agostbiro@users.noreply.github.com> Co-authored-by: Wodann <Wodann@users.noreply.github.com> Co-authored-by: Piotr Galar <piotr.galar@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Franco Victorio <victorio.franco@gmail.com> * chore: Restore old test.ts and remove some `as any` conversions * WIP: Benchmark with patched Hardhat to use new EDR internals * Revert "WIP: Benchmark with patched Hardhat to use new EDR internals" This reverts commit ac2e012. * Remove the gen-hardhat-patches script --------- Co-authored-by: Agost Biro <5764438+agostbiro@users.noreply.github.com> Co-authored-by: Wodann <Wodann@users.noreply.github.com> Co-authored-by: Piotr Galar <piotr.galar@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Franco Victorio <victorio.franco@gmail.com>

fvictorio added 5 commits August 20, 2024 11:40

Fix pnpm warning

52433b1

Add syncpakc

0fa20ab

Add syncpack

327f50d

Run syncpack format

b33ac11

Run syncpack on CI

695a61f

Add setup-node action

ce30c1e

fvictorio force-pushed the js-improvements branch from 303e16f to ce30c1e Compare August 21, 2024 07:31

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 07:31 — with GitHub Actions Error

fvictorio added the no changeset needed This PR doesn't require a changeset label Aug 21, 2024

fvictorio added 2 commits August 21, 2024 09:44

Fixes related to upgrading prettier

88a320e

Run prettier

b072058

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 07:45 — with GitHub Actions Error

fvictorio added 2 commits August 21, 2024 09:47

Add setup-rust action

1bf2427

Run prettier in edr_napi

213dad3

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 07:49 — with GitHub Actions Error

fvictorio added 2 commits August 21, 2024 10:00

Add lint scripts to packages

513f246

Run prettier in crates/tools/js/benchmark

8eadf7c

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 08:01 — with GitHub Actions Failure

fvictorio added 2 commits August 21, 2024 10:49

Port benchmark code to typescript

9bfb4c9

Add eslint to all packages

16ef9ba

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 08:57 — with GitHub Actions Error

Upgrade @types/node to v20

e183c79

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 09:01 — with GitHub Actions Error

fvictorio requested review from agostbiro, Xanewok and Wodann August 21, 2024 09:10

Fix broken build:edr script

176e83c

fvictorio had a problem deploying to github-action-benchmark August 21, 2024 09:12 — with GitHub Actions Failure

fvictorio changed the title ~~(WIP) chore: tooling improvement~~ chore: tooling improvement Aug 21, 2024

Resolve benchmark output path

46d0eb3

fvictorio temporarily deployed to github-action-benchmark August 21, 2024 11:36 — with GitHub Actions Inactive

fvictorio changed the title ~~chore: tooling improvement~~ chore: js tooling improvements Aug 21, 2024

Wodann approved these changes Aug 21, 2024

View reviewed changes

agostbiro reviewed Aug 21, 2024

View reviewed changes

agostbiro approved these changes Aug 21, 2024

View reviewed changes

fvictorio added this pull request to the merge queue Aug 21, 2024

Merged via the queue into main with commit 87f2833 Aug 21, 2024
41 checks passed

fvictorio deleted the js-improvements branch August 21, 2024 16:15

github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: js tooling improvements #609

chore: js tooling improvements #609

fvictorio commented Aug 21, 2024 •

edited

Loading

changeset-bot bot commented Aug 21, 2024 •

edited

Loading

Wodann left a comment

Wodann Aug 21, 2024

fvictorio Aug 21, 2024

agostbiro Aug 21, 2024 •

edited

Loading

fvictorio Aug 21, 2024

agostbiro Aug 21, 2024

chore: js tooling improvements #609

chore: js tooling improvements #609

Conversation

fvictorio commented Aug 21, 2024 • edited Loading

Syncpack

setup-node and setup-rust actions

Migrate tools/js/benchmark to typescript

Better linting in general

changeset-bot bot commented Aug 21, 2024 • edited Loading

⚠️ No Changeset found

Wodann left a comment

Choose a reason for hiding this comment

Wodann Aug 21, 2024

Choose a reason for hiding this comment

fvictorio Aug 21, 2024

Choose a reason for hiding this comment

agostbiro Aug 21, 2024 • edited Loading

Choose a reason for hiding this comment

fvictorio Aug 21, 2024

Choose a reason for hiding this comment

agostbiro Aug 21, 2024

Choose a reason for hiding this comment

fvictorio commented Aug 21, 2024 •

edited

Loading

changeset-bot bot commented Aug 21, 2024 •

edited

Loading

agostbiro Aug 21, 2024 •

edited

Loading