Dynamic client side envs #79
Conversation
|
I'm still on the fence whether we should do this, @grod220 any thoughts? |
| const envs = whitelist.reduce( | ||
| (envs: Record<string, string>, key) => ({ | ||
| ...envs, | ||
| [key]: process.env[key] ?? '', |
There was a problem hiding this comment.
question: why using context instead of just exporting envs as consts?
There was a problem hiding this comment.
We need to store it somewhere since we receive it from the server-side, this is a temporary solution as we're still debating the state manager.
|
|
||
| export async function getServerSideProps() { | ||
| const envs = getClientSideEnvs(); |
There was a problem hiding this comment.
suggestion: instead of calling this function on every page, expose selected environmental variables with NEXT_PUBLIC_ prefix. See nextjs docs about runtime envs.
There was a problem hiding this comment.
NEXT_PUBLIC_ envs will be compiled within the build unfortunately. So you can not provide an env var starting with NEXT_PUBLIC_ after the build is complete and let nextjs use that.
| @@ -0,0 +1,13 @@ | |||
| export function getClientSideEnvs() { | |||
| const whitelist: string[] = ['PENUMBRA_CHAIN_ID', 'PENUMBRA_CUILOA_URL']; | |||
There was a problem hiding this comment.
suggestion: add .env.example file to env. It shouldn't have any values, just keys with comments on what they do, and where to get them from
|
It's tough for me to review this with all the formatting changes. Could you open another PR that just does the formatting (and adds CI/CD checks for prettier)? |
I didn't wanna do it, but I did it: #80 |
|
Closing this one. |
This isn't the formatting change PR though right? I think those changes are important and there should be a CI/CD test like in penumbra-zone/web. |
|
If we merged that separately, this PR would have automatically been cleared up |
As per @conorsch 's request, this adds support to set envs dynamically post-build.
envsfrom getServerSideProps as props