-
Notifications
You must be signed in to change notification settings - Fork 248
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: modularization and bundling enhancements #1353
Conversation
🦋 Changeset detectedLatest commit: f027f1b The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
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 |
ae7ef28
to
e64c493
Compare
d8de27f
to
d3596b0
Compare
32d79fa
to
68d26bf
Compare
ecdaf67
to
e5e4203
Compare
8905f93
to
015c916
Compare
… add type dependencies
6649e50
to
60fddf3
Compare
avoids short-term breaking changes for packages outdated `near-api-js` peer dependencies
5a694b6
to
096c8d4
Compare
096c8d4
to
2fdb57d
Compare
|
||
/** | ||
* PublicKey representation that has type and bytes of the key. | ||
*/ | ||
export class PublicKey extends Assignable { | ||
export class PublicKey extends Enum { |
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.
@gtsonevv @gagdiez I wanted to get a sanity check on this since it should have always been an Enum
but I think worked just because Assignable
is flexible. It now needs to extend Enum
in order to follow the new rules around class fields and how that impacts serialization (see PR description). Doesn’t seem to cause issues but if you have concerns/insights please let me know.
Note that this also impacts Signature
in the same way, which was similarly changed to support both ED25519 and SECP256K1 curves.
Pre-flight checklist
pnpm changeset
to create achangeset
JSON document appropriate for this change.Motivation
This PR contains several enhancements intended to modernize and simplify usage of the
@near-js
packages:@near-js
packages only,near-api-js
was too much of a headache)One of the more substantial changes required to target
es2022
was the removal of theAssignable
class and modification of theEnum
class, both used primarily in@near-js/transactions
for (de)serialization of actions. With public class fields, the behavior around setting class fields from the base constructor has changed and the dynamic setting logic inAssignable
is no longer valid. However, this provides a good opportunity to define class properties per action, which while redundant, provides more intuitive typing support. See relevant SO question.While
Assignable
could be removed*, I effectively ran into the opposite problem withEnum
. Serialization breaks if any of the following are violated:Enum
-derived classes must inherit fromEnum
, so we can't just deleteEnum
and implement same logic in the classesenum
field cannot be defined as a public field onEnum
nor set from within theEnum
constructor - the inheriting class must set it, so I've updated it to beabstract enum: string;
Enum
constructor must still take the properties and set them on itself, despite the class constructor explicitly setting them... this is what appeared to breakAssignable
-derived classes 😑I'd need to look into this further to understand what's exactly going on, but the current implementation works and is only a little less confusing than the existing one. *
Assignable
will remain for backward compatibility reasons, as outdated NAJ peer dependencies in Wallet Selector dependencies break otherwise.Test Plan
Due to the scope of these changes, a
next
(near-api-js@next) tag has been created in NPM referencing packages built from this branch. Long term thenext
tag will require a dedicated build step in order to remain current. Wallet Selector update looks good (React example works): near/wallet-selector#1183The attempt at an
esbuild
bundle was missing some functionality and will require further debugging to get working (see branchfeat/dist-bundling
for original implementation). For now we will be deprecating as part of this major release in favor of using ES modules in browser.So going from supporting this:
To relying on modern (starting ~2017-18) browsers supporting modules:
This will reduce bundle size substantially for most use cases and is expected to be an easy lift for applications using the old format.
Related issues/PRs