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

feat: update borsh-js to v1.0.0 #1168

Merged
merged 9 commits into from
Oct 31, 2023
Merged

feat: update borsh-js to v1.0.0 #1168

merged 9 commits into from
Oct 31, 2023

Conversation

gagdiez
Copy link
Contributor

@gagdiez gagdiez commented Jul 26, 2023

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this changes code in a published package: I have run pnpm changeset to create a changeset JSON document appropriate for this change.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

We have developed a new version of borsh-js which is agnostic to the Class being (de)serialized. This means that two "deep equal" objects can be (de)Serialized using the same Schema, no matter their origin or particular Class.

Test Plan

I have updated the dependency of borsh-js and changed the code as minimal as possible until all the tests passed again.

Related issues/PRs

Fixes #1167

gagdiez added 3 commits July 26, 2023 18:10
We have been working on a new version of borsh-js, this commit
updates all the files that depend on borsh-js. This update is
a breaking change, since the objects schemas (which are exported)
are completely different from v0.7.0 (before) to v1.0.1 (now).

The main benefit of updating borsh-js is that the new version
is agnostic to the class being serialized, meaning that two
"deep-equal" instances of different classes can be serialized
using the same Schema. This means that near-api-js will be
able to (de)serialize objects created across different
library versions (which currently is not possible).
@gagdiez gagdiez added the WIP label Jul 26, 2023
@gagdiez gagdiez requested a review from andy-haynes July 26, 2023 16:42
@changeset-bot
Copy link

changeset-bot bot commented Jul 26, 2023

🦋 Changeset detected

Latest commit: 54f6708

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
near-api-js Major
@near-js/accounts Major
@near-js/biometric-ed25519 Major
@near-js/crypto Major
@near-js/transactions Major
@near-js/wallet-account Major
@near-js/cookbook Patch
@near-js/keystores-browser Patch
@near-js/keystores-node Patch
@near-js/keystores Patch
@near-js/signers Patch
@near-js/providers Patch

Not sure what this means? Click here to learn what changesets are.

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

gagdiez added 5 commits July 28, 2023 12:50
The new version of Borsh has been released without the `baseDecode`
and `baseEncode` methods. I have migrated these methods as they
were from `borsh` to the `near-api-js/utils/format` package.
@gagdiez gagdiez changed the title WIP: feat: update borsh-js to v1.0.1 feat: update borsh-js to v1.0.0 Aug 4, 2023
@gagdiez gagdiez removed the WIP label Aug 4, 2023
@gagdiez
Copy link
Contributor Author

gagdiez commented Aug 4, 2023

@andy-haynes this PR is ready to be reviewed.

The most important thing to take into account is that the new version of borsh uses different SCHEMAS, and near-api-js exports both serialize and deserialize from borsh. This meant that the change in this PR is a breaking change.

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

Looks good to me

@frol frol enabled auto-merge (squash) October 31, 2023 19:46
@frol frol merged commit 61349ae into near:master Oct 31, 2023
1 check passed
@github-actions github-actions bot mentioned this pull request Oct 31, 2023
Comment on lines +2 to +7
"near-api-js": major
"@near-js/accounts": major
"@near-js/biometric-ed25519": major
"@near-js/crypto": major
"@near-js/transactions": major
"@near-js/wallet-account": major
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gagdiez This caused packages to be released as 1.0.0 release instead of 0.x 😐

I just wanted to mention it for future reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Class is missing in schema & Object is missing in schema
2 participants