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.

Sign up for GitHub

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

Jump to bottom

NPMify! #91

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

NPMify! #91

wants to merge 4 commits into from

Conversation

Lev-Stambler
Copy link
Collaborator

  • Publish packages to NPM script
  • Move token <--> indivisible units to utils
  • Move Pair to interfaces cause circular dependencies
  • Add ChainAsType to improve ease of use for external apps using the library

@Lev-Stambler Lev-Stambler requested a review from Sladuca May 14, 2021 20:40
@vercel
Copy link

vercel bot commented May 14, 2021

Deployment failed with the following error:

The most recent charge for your active payment method has failed. Please update it here: https://vercel.com/teams/baf-wallet/settings/billing.

Sladuca
Sladuca requested changes May 16, 2021
View reviewed changes
Copy link
Contributor

@Sladuca Sladuca left a comment

Choose a reason for hiding this comment

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

Looks great! Just one small change and possibly another - see comments for more details

libs/interfaces/src/lib/chains.ts

export type ChainAsType = 'near' | 'ethereum';
Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly does this do? Is this for code I wrote a long time ago that expected it to be a type and not a 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.

This is just to make the api easier for the library. Like you can do tokenInfo('near') instead of Chains.Near

libs/interfaces/dist/libs/interfaces/package.json
"@toruslabs/torus-direct-web-sdk": "^4.11.1"
},
"peerDependencies": {}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an exports property here so that we can import it like @baf-wallet/core/interfaces instead of having to do @baf-wallet/core/src/interfaces to get interfaces (and similarly for other non-default libs)?

see https://webpack.js.org/guides/package-exports/ for more information

libs/core-publishable/src/index.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sladuca Sladuca Sladuca requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Lev-Stambler @Sladuca