-
Notifications
You must be signed in to change notification settings - Fork 25
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
eden-subchain-client #556
eden-subchain-client #556
Conversation
@@ -10,7 +10,7 @@ import { | |||
ClientStatus, | |||
ServerMessage, | |||
sanitizeClientStatus, | |||
} from "@edenos/common/dist/subchain/SubchainProtocol"; | |||
} from "@edenos/eden-subchain-client/dist/SubchainProtocol"; |
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.
question: any reason to force the usage of /dist/
? It forces us to recompile the package if we change something in the eden-subchain-client
which defeats the purpose of our local lerna packages structure... eg: we can change @edenos/common
as much as we want without recompiling and our ide and webapp hot live reload works.
ps: I didn't catch that before :(
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.
Tree shaking, especially with dependances. e.g. nodejs users which aren't doing server-side generation shouldn't pull in react.
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.
how tree shaking will be different here if we would be using @edenos/eden-subchain-client/SubchainProtocol
(no /dist
)?
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.
@edenos/eden-subchain-client/SubchainProtocol
produces
Cannot find module '@edenos/eden-subchain-client/SubchainProtocol' or its corresponding type declarations
@edenos/eden-subchain-client
works but pulls in too much
@@ -1,4 +1,4 @@ | |||
import { EdenSubchain } from "@edenos/common/dist/subchain"; | |||
import { EdenSubchain } from "@edenos/eden-subchain-client/dist/EdenSubchain"; |
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.
- same as above dist question
@@ -0,0 +1,41 @@ | |||
# EdenOS Subchain Client |
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.
praise: awesome documentations! prop to amazing ascii diagrams!!! 🤣
stateUrl: string, | ||
wsUrl: string, | ||
slowMo: boolean | ||
options: UseSubchainClientOptions |
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.
praise: beautiful refactor!
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.
lgtm. I would just try to remove the dependencies from /dist if it's possible.
Publishing an npm package for running queries against eden history