-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Validate env vars with Zod #2362
base: main
Are you sure you want to change the base?
Changes from all commits
ddd10d1
90baa94
b5c3c63
7afc4f0
b4e2362
465856c
5c329a4
e04aac3
1928b6f
e231dda
f431153
c8b2120
08304fe
a509762
9d618ea
7333b25
80baf9c
fb899a0
7593ac2
42d121c
f877543
025d185
1da4542
67de093
ee7c6de
7b0fedd
598eeab
4732a27
fcdc0d6
68e99ba
c740e4e
88d7ea9
107fa51
42e9c78
9dc30c1
4766a94
ba46aa8
920cc90
835e17a
d275338
73e5e11
41e24be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,15 +55,15 @@ WORKDIR /app | |
# Copying the top level 'node_modules' because it contains the Prisma packages | ||
# necessary for migrating the database. | ||
COPY --from=server-builder /app/node_modules ./node_modules | ||
# Copying the SDK because 'validate-env.mjs' executes independent of the bundle | ||
# Copying the SDK because the server bundle doesn't bundle the SDK | ||
# and references the 'wasp' package. | ||
COPY --from=server-builder /app/.wasp/out/sdk .wasp/out/sdk | ||
# Copying 'server/node_modules' because 'validate-env.mjs' executes independent | ||
# of the bundle and references the dotenv package. | ||
# Copying 'server/node_modules' because we require dotenv package to | ||
# load environment variables | ||
# TODO: replace dotenv with native Node.js environment variable loading | ||
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. Why is that, what is wrong with dotenv? Node.js now has native support for it that is equally good? 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. It's always nicer to use natively supported methods than a package to do the same thing. It's available since Node.js 20 and we should probably go for it when Node.js 20 becomes our minimum version. 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. We should probabl make nodejs 20 our minimum version any moment hm. Ok, so it does do the same thing? If so, great. |
||
COPY --from=server-builder /app/.wasp/build/server/node_modules .wasp/build/server/node_modules | ||
COPY --from=server-builder /app/.wasp/build/server/bundle .wasp/build/server/bundle | ||
COPY --from=server-builder /app/.wasp/build/server/package*.json .wasp/build/server/ | ||
COPY --from=server-builder /app/.wasp/build/server/scripts .wasp/build/server/scripts | ||
infomiho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
COPY db/ .wasp/build/db/ | ||
EXPOSE ${PORT} | ||
WORKDIR /app/.wasp/build/server | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{{={= =}=}} | ||
import * as z from 'zod' | ||
|
||
import { ensureEnvSchema } from '../env/index.js' | ||
|
||
const clientEnvSchema = z.object({ | ||
REACT_APP_API_URL: z | ||
.string({ | ||
required_error: 'REACT_APP_API_URL is required', | ||
}) | ||
.default('{= defaultServerUrl =}') | ||
}) | ||
|
||
// PUBLIC API | ||
export const env = ensureEnvSchema(import.meta.env, clientEnvSchema) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import * as z from 'zod' | ||
|
||
const redColor = '\x1b[31m' | ||
|
||
export function ensureEnvSchema<Schema extends z.ZodTypeAny>( | ||
data: unknown, | ||
schema: Schema | ||
): z.infer<Schema> { | ||
try { | ||
return schema.parse(data) | ||
} catch (e) { | ||
if (e instanceof z.ZodError) { | ||
const errorOutput = ['', '══ Env vars validation failed ══', ''] | ||
for (const error of e.errors) { | ||
errorOutput.push(` - ${error.message}`) | ||
} | ||
errorOutput.push('') | ||
errorOutput.push('════════════════════════════════') | ||
console.error(redColor, errorOutput.join('\n')) | ||
throw new Error('Error parsing environment variables') | ||
} else { | ||
throw e | ||
} | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,19 @@ | ||
import { OAuth2Provider, OAuth2ProviderWithPKCE } from "arctic"; | ||
|
||
export function defineProvider< | ||
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE, | ||
Env extends Record<string, string> | ||
OAuthClient extends OAuth2Provider | OAuth2ProviderWithPKCE | ||
>({ | ||
id, | ||
displayName, | ||
env, | ||
oAuthClient, | ||
}: { | ||
id: string; | ||
displayName: string; | ||
env: Env; | ||
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. Instead of OAuth env vars having their special validation and living in a special place, they are now used directly from the |
||
oAuthClient: OAuthClient; | ||
}) { | ||
return { | ||
id, | ||
displayName, | ||
env, | ||
oAuthClient, | ||
}; | ||
} |
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.
Are you sure? Why wouldn't it bundle the SDK? It's a dependency like any other (and it's copied with the node modules anyway).
Also, there's the second part of the old comment (below).
I know I wrote the original comment but I am not sure what I meant. But, I think I remember specifically needing it for
validate-env.mjs
.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.
Rollup doesn't bundle the
wasp/*
deps because they are considered "external" deps. I tried making them "internal" (adjust the regex for external deps) and it still didn't want to bundle them. Maybe because they are fromnode_modules
. So, yep, we still need to copy over the SDK in the build context.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.
Can you add the explanation (or a link to this discussion in the comment)?