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: modularization and bundling enhancements #1353

Merged
merged 45 commits into from
Sep 4, 2024

Conversation

andy-haynes
Copy link
Collaborator

@andy-haynes andy-haynes commented Jun 18, 2024

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

This PR contains several enhancements intended to modernize and simplify usage of the @near-js packages:

  • packages are now exported as modules, enabling tree shaking (@near-js packages only, near-api-js was too much of a headache)
  • dependencies have been updated to isomorphic packages wherever possible
  • global references have been removed, allowing frictionless integration with modern frameworks like Vite
  • tests have been moved to TS, enabling direct import and debugging of source TS
  • Node and pnpm versions have been updated to 20.15.0 and 9.4.0, respectively

One of the more substantial changes required to target es2022 was the removal of the Assignable class and modification of the Enum 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 in Assignable 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 with Enum. Serialization breaks if any of the following are violated:

  • both Enum-derived classes must inherit from Enum, so we can't just delete Enum and implement same logic in the classes
  • the enum field cannot be defined as a public field on Enum nor set from within the Enum constructor - the inheriting class must set it, so I've updated it to be abstract enum: string;
  • the Enum constructor must still take the properties and set them on itself, despite the class constructor explicitly setting them... this is what appeared to break Assignable-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 the next tag will require a dedicated build step in order to remain current. Wallet Selector update looks good (React example works): near/wallet-selector#1183

The attempt at an esbuild bundle was missing some functionality and will require further debugging to get working (see branch feat/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:

<script type="text/javascript" src="https://cdn.jsdelivr.net/npm/[email protected]/dist/near-api-js.min.js"></script>
<script type="module">
  const { providers: { JsonRpcProvider } } = window.nearApi;
  ...
</script>

To relying on modern (starting ~2017-18) browsers supporting modules:

<script type="module">
  import { providers } from 'https://esm.sh/near-api-js@next';
  const { JsonRpcProvider } = providers;
  ...
</script>

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

Copy link

changeset-bot bot commented Jun 18, 2024

🦋 Changeset detected

Latest commit: f027f1b

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

This PR includes changesets to release 15 packages
Name Type
@near-js/accounts Minor
@near-js/biometric-ed25519 Minor
@near-js/crypto Minor
@near-js/iframe-rpc Minor
@near-js/keystores Minor
@near-js/keystores-browser Minor
@near-js/keystores-node Minor
near-api-js Major
@near-js/providers Major
@near-js/signers Minor
@near-js/transactions Minor
@near-js/types Minor
@near-js/utils Major
@near-js/wallet-account Minor
@near-js/cookbook 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

@andy-haynes andy-haynes changed the base branch from master to feat/multi-contract-keystore-rebased August 19, 2024 22:47
@andy-haynes andy-haynes force-pushed the feat/bundling-enhancements branch 2 times, most recently from ecdaf67 to e5e4203 Compare August 19, 2024 23:15
Base automatically changed from feat/multi-contract-keystore-rebased to master August 20, 2024 18:45
@andy-haynes andy-haynes force-pushed the feat/bundling-enhancements branch 2 times, most recently from 6649e50 to 60fddf3 Compare August 20, 2024 21:38
avoids short-term breaking changes for packages outdated `near-api-js` peer dependencies
@andy-haynes andy-haynes marked this pull request as ready for review August 28, 2024 00:45
mpeterdev
mpeterdev previously approved these changes Aug 28, 2024

/**
* PublicKey representation that has type and bytes of the key.
*/
export class PublicKey extends Assignable {
export class PublicKey extends Enum {
Copy link
Collaborator Author

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.

@andy-haynes andy-haynes merged commit 6ceb62e into master Sep 4, 2024
1 check passed
@andy-haynes andy-haynes deleted the feat/bundling-enhancements branch September 4, 2024 00:11
@github-actions github-actions bot mentioned this pull request Sep 4, 2024
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.

2 participants