Skip to content
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

light-client-js: remove the syn version pin #1242

Merged
merged 6 commits into from
Dec 8, 2022

Conversation

mzabaluev
Copy link
Contributor

The bug this used to work around is closed and the problem
no longer seems to persist.

The bug this used to work around is closed and the problem
no longer seems to persist.
@mzabaluev mzabaluev added dependencies Pull requests that update a dependency file wasm labels Dec 5, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2022

Codecov Report

Merging #1242 (84b9cc6) into main (e9c41de) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

@@           Coverage Diff           @@
##            main   #1242     +/-   ##
=======================================
- Coverage   64.4%   64.4%   -0.1%     
=======================================
  Files        245     245             
  Lines      21485   21489      +4     
=======================================
  Hits       13842   13842             
- Misses      7643    7647      +4     
Impacted Files Coverage Δ
light-client-js/src/lib.rs 1.7% <0.0%> (-0.2%) ⬇️
testgen/src/vote.rs 84.8% <0.0%> (-0.9%) ⬇️
testgen/src/commit.rs 90.6% <0.0%> (-0.7%) ⬇️
light-client-verifier/src/types.rs 38.7% <0.0%> (ø)
tendermint/src/node.rs 67.3% <0.0%> (+0.1%) ⬆️
testgen/src/header.rs 84.0% <0.0%> (+0.5%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

To remove deprecation warnings and do the right thing,
use serde-wasm-bindgen utilities for serializing and deserializing
Rust types to and from JsValue.
light-client-js/src/lib.rs Outdated Show resolved Hide resolved
Accept also signed integers as these are produced from JSON
by serde-wasm-bindgen.
Now that deserialization likes to take ownership of our JS values,
it does not make sense to pass them by reference from JavaScript
only to clone them immediately.
This is not a breaking change for the JavaScript API.
Copy link
Member

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

LGTM!

Would this be considered a breaking change, given the dependency update?

@mzabaluev
Copy link
Contributor Author

Would this be considered a breaking change, given the dependency update?

I don't think so. A wasm bundle has all its Rust dependencies baked in, and the JavaScript signature of the entry point did not change either.

That said, I need to give it a suitable changelog entry.

@mzabaluev mzabaluev merged commit 60d003b into main Dec 8, 2022
@mzabaluev mzabaluev deleted the mikhail/remove-syn-pin-for-wasm-pack branch December 8, 2022 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants