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

init react-hooks #1

Merged
merged 15 commits into from
Oct 27, 2023
Merged

init react-hooks #1

merged 15 commits into from
Oct 27, 2023

Conversation

nickadamson
Copy link
Collaborator

@nickadamson nickadamson commented Oct 17, 2023

Implements VAL-447

waiting on sdk v0.0.2-alpha.0

@nickadamson nickadamson self-assigned this Oct 17, 2023
@nickadamson nickadamson marked this pull request as draft October 21, 2023 04:42
@0xAlcibiades 0xAlcibiades marked this pull request as ready for review October 21, 2023 19:41
@albertocevallos
Copy link
Contributor

albertocevallos commented Oct 23, 2023

Let's add a README with the following:

  • Version
  • License
  • How to install
  • How to use the package
  • How to run in development

I'll push an example below.

@albertocevallos
Copy link
Contributor

albertocevallos commented Oct 23, 2023

Another potential consideration, if we're releasing the repo as a package - might be better to export hooks, context and lib as a class with arguments for custom RPC providers, proto server, etc - as opposed to exporting those folders raw.

TLDR: It allows devs to control the package more easily.

Other suggestions:

  • Releasing pkg as tag with alpha nomenclature until ready for release
  • Add tsconfig or package.json rule to import folders in isolation

src/index.ts Outdated
@@ -0,0 +1,3 @@
export * from './context';
export * from './hooks';
Copy link
Contributor

Choose a reason for hiding this comment

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

To support tree shaking (and prevent importing unnecessary hooks) its best to import each hook manually.

src/index.ts Outdated
@@ -0,0 +1,3 @@
export * from './context';
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed earlier, we should use export default ValoremProvider from "/..." for devs to config their RPCs, etc

@nickadamson nickadamson marked this pull request as draft October 24, 2023 02:14
@0xAlcibiades 0xAlcibiades marked this pull request as ready for review October 27, 2023 01:24
@0xAlcibiades 0xAlcibiades merged commit 3093820 into main Oct 27, 2023
1 check passed
@0xAlcibiades 0xAlcibiades deleted the init branch October 27, 2023 01:24
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.

3 participants