edr
edr copied to clipboard
feat: add Optimism as separate N-API package
This PR restructures the edr_napi_core
and edr_napi
to be able to share code with the new edr_napi_optimism
package.
I ran into one problem that I was able to solve. Namely, when a crate that generates NAPI bindings is added as a dependency to another crate that generates NAPI bindings, the test target of the dependant crate will throw linker errors. This is a known issue. I was only able to work around it by adding the recommended rustflags to both the workspace- and project-level .cargo/config.toml
. If I only added it to the project, the cargo test --all-features --all-targets --workspace
command would still throw the linker errors.
There is currently still a bug in the edr_napi_optimism
JS tests. In the background, the hyper
Client is sporadically trying to execute a future using its executor. By default, this executor uses tokio::spawn
but since the edr_optimism
NPM package doesn't have a tokio runtime, this panics.
thread '
' panicked at /home/vscode/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-0.14.28/src/common/exec.rs:49:21: there is no reactor running, must be called from the context of a Tokio 1.x runtime note: run with RUST_BACKTRACE=1
environment variable to display a backtrace
The solution is to specify a custom executor, but reqwest always uses the Tokio executor. An issue exists to add this executor, but a previous PR was rejected because it was deemed unnecessary.
For now, my proposal would be to merge the changes to the project layout and move forward with the release of the GenericChainSpec
, as edr_optimism
won't be exposed anyway.
A future issue can then:
- fork
reqwest
- add setting of the executor
- set the executor with the
tokio::Runtime
of ouredr
NPM package's runtime
Could you please pay special attention to these two aspects when reviewing?