-
Notifications
You must be signed in to change notification settings - Fork 0
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
cleanup: clean up a bit of the mess i created #37
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,17 @@ | ||
#!/usr/bin/env node | ||
import "dotenv/config"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not quite sure if it should be here ... probably not and one should export ALCHEMY_API_KEY before running the cli or put the api key as parameter on the script. Didn't want to add commander or similar for such a simple task though and working with pure args in node is pain :/ I'm conflicted. |
||
import { ChainId, getNetworkEnv, getRPCUrl } from "./lib"; | ||
|
||
(function print() { | ||
let env = ""; | ||
let toml = ""; | ||
Object.entries(ChainId).map(([key, chainId]) => { | ||
const networkEnv = getNetworkEnv(chainId); | ||
const rpc = getRPCUrl(chainId, { alchemyKey: process.env.ALCHEMY_API_KEY }); | ||
const rpc = getRPCUrl(chainId, { | ||
alchemyKey: process.env.ALCHEMY_API_KEY, | ||
quicknodeToken: process.env.QUICKNODE_TOKEN, | ||
quicknodeEndpointName: process.env.QUICKNODE_ENDPOINT_NAME, | ||
}); | ||
env += `${networkEnv}=${rpc || ""}\n`; | ||
toml += `${key}="\${${networkEnv}}"\n`; | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import { networkMap } from "./alchemyIds"; | ||
import { ChainId, ChainList } from "./chainIds"; | ||
import { publicRPCs } from "./public"; | ||
import { publicRPCs } from "./publicRPCs"; | ||
import { quicknodeNetworkMap } from "./quicknodeIds"; | ||
|
||
type SupportedChainIds = (typeof ChainId)[keyof typeof ChainId]; | ||
|
@@ -141,4 +141,4 @@ export const getRPCUrl = ( | |
} catch (e) {} | ||
}; | ||
|
||
export { ChainId, ChainList }; | ||
export { ChainId, ChainList, type SupportedChainIds }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. noticed it would be useful to have this type. |
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.
why is everything
devDependencies
?When using as a lib, i would expect viem to be a peerDependency.
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.
Because of how tsup works. DevDependencies are bundled up automatically and as we're only using the deps for some constants, IMO that is much nicer for downstream users.
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.
This is particularly useful for actions-usage, where no dependency resolution takes place.
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.
i see, but currently, this means then when using as js lib it would contain the inlined viem + the viem the package uses?
It#s not a big problem as currently we only use this is node modules were pkg size does not matter, but perhaps that's sth we should look into down the line.
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.
Depending on if tree shake works, I'd say.. does the whole view land in there or the constants we need?