-
-
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 31 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 |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{{={= =}=}} | ||
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 =}') | ||
}) | ||
|
||
export const env = ensureEnvSchema(import.meta.env, clientEnvSchema) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
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) { | ||
// TODO: figure out how to output the error message in a better way | ||
infomiho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (e instanceof z.ZodError) { | ||
console.error() | ||
console.error(redColor, '╔═════════════════════════════╗'); | ||
console.error(redColor, '║ Env vars validation failed ║'); | ||
console.error(redColor, '╚═════════════════════════════╝'); | ||
console.error() | ||
for (const error of e.errors) { | ||
console.error(redColor, `- ${error.message}`) | ||
} | ||
console.error() | ||
console.error(redColor, '═══════════════════════════════'); | ||
throw new Error('Error parsing environment variables') | ||
} else { | ||
throw e | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// Used for internal Wasp development only, not copied to generated app. | ||
module.exports = { | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
trailingComma: 'es5', | ||
tabWidth: 2, | ||
semi: false, | ||
singleQuote: true, | ||
} |
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, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,94 +1,47 @@ | ||
{{={= =}=}} | ||
import merge from 'lodash.merge' | ||
import { env } from './env.js' | ||
import { stripTrailingSlash } from '../universal/url.js' | ||
|
||
import { stripTrailingSlash } from "../universal/url.js"; | ||
type NodeEnv = typeof env.NODE_ENV | ||
|
||
const nodeEnv = process.env.NODE_ENV ?? 'development' | ||
|
||
// TODO: | ||
// - Use dotenv library to consume env vars from a file. | ||
// - Use convict library to define schema and validate env vars. | ||
// https://codingsans.com/blog/node-config-best-practices | ||
|
||
type BaseConfig = { | ||
type Config = { | ||
env: NodeEnv; | ||
isDevelopment: boolean; | ||
port: number; | ||
databaseUrl: string; | ||
frontendUrl: string; | ||
serverUrl: string; | ||
allowedCORSOrigins: string | string[]; | ||
{=# isAuthEnabled =} | ||
auth: { | ||
jwtSecret: string | undefined; | ||
jwtSecret: string; | ||
} | ||
{=/ isAuthEnabled =} | ||
} | ||
|
||
type CommonConfig = BaseConfig & { | ||
env: string; | ||
isDevelopment: boolean; | ||
port: number; | ||
databaseUrl: string | undefined; | ||
} | ||
const frontendUrl = stripTrailingSlash(env.WASP_WEB_CLIENT_URL) | ||
const serverUrl = stripTrailingSlash(env.WASP_SERVER_URL) | ||
|
||
type EnvConfig = BaseConfig & { | ||
frontendUrl: string; | ||
serverUrl: string; | ||
const allowedCORSOriginsPerEnv: Record<NodeEnv, string | string[]> = { | ||
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. Default CORS values per NODE_ENV, same as before. |
||
development: '*', | ||
production: [frontendUrl] | ||
} | ||
|
||
type Config = CommonConfig & EnvConfig | ||
|
||
const config: { | ||
all: CommonConfig, | ||
development: EnvConfig, | ||
production: EnvConfig, | ||
} = { | ||
all: { | ||
env: nodeEnv, | ||
isDevelopment: nodeEnv === 'development', | ||
port: process.env.PORT ? parseInt(process.env.PORT) : {= defaultServerPort =}, | ||
databaseUrl: process.env.{= databaseUrlEnvVarName =}, | ||
allowedCORSOrigins: [], | ||
{=# isAuthEnabled =} | ||
auth: { | ||
jwtSecret: undefined | ||
} | ||
{=/ isAuthEnabled =} | ||
}, | ||
development: getDevelopmentConfig(), | ||
production: getProductionConfig(), | ||
} | ||
|
||
const resolvedConfig: Config = merge(config.all, config[nodeEnv]) | ||
// PUBLIC API | ||
export default resolvedConfig | ||
|
||
function getDevelopmentConfig(): EnvConfig { | ||
const frontendUrl = stripTrailingSlash(process.env.WASP_WEB_CLIENT_URL ?? '{= defaultClientUrl =}'); | ||
const serverUrl = stripTrailingSlash(process.env.WASP_SERVER_URL ?? '{= defaultServerUrl =}'); | ||
return { | ||
// @ts-ignore | ||
frontendUrl, | ||
// @ts-ignore | ||
serverUrl, | ||
allowedCORSOrigins: '*', | ||
{=# isAuthEnabled =} | ||
auth: { | ||
jwtSecret: 'DEVJWTSECRET' | ||
} | ||
{=/ isAuthEnabled =} | ||
const allowedCORSOrigins = allowedCORSOriginsPerEnv[env.NODE_ENV] | ||
|
||
const config: 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. Simplified the creation of the 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. Do we loose anyting this way? Anythihg int he old appraoch that was beneficial from having that split? Why was that split there otherwise, I do think it had some benefits possibly? 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. I think the previous approach we clever since it envisioned many different options for dev and for production, but it in the end, we only vary the CORS origins between dev and production. So, it seemed like an overkill and I know I had these complex types to capture the different objects. But when I looked closer, it turned out this simple 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. Ok, makes sense, we can always modify it if needed! |
||
frontendUrl, | ||
serverUrl, | ||
allowedCORSOrigins, | ||
env: env.NODE_ENV, | ||
isDevelopment: env.NODE_ENV === 'development', | ||
port: env.PORT, | ||
databaseUrl: env.{= databaseUrlEnvVarName =}, | ||
{=# isAuthEnabled =} | ||
auth: { | ||
jwtSecret: env.JWT_SECRET | ||
} | ||
{=/ isAuthEnabled =} | ||
} | ||
|
||
function getProductionConfig(): EnvConfig { | ||
const frontendUrl = stripTrailingSlash(process.env.WASP_WEB_CLIENT_URL); | ||
const serverUrl = stripTrailingSlash(process.env.WASP_SERVER_URL); | ||
return { | ||
// @ts-ignore | ||
frontendUrl, | ||
// @ts-ignore | ||
serverUrl, | ||
// @ts-ignore | ||
allowedCORSOrigins: [frontendUrl], | ||
{=# isAuthEnabled =} | ||
auth: { | ||
jwtSecret: process.env.JWT_SECRET | ||
} | ||
{=/ isAuthEnabled =} | ||
} | ||
} | ||
// PUBLIC API | ||
export default config |
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 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 comment
The 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 comment
The 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.