Skip to content
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

feat: add typesafe client side env variables #209

Merged
merged 14 commits into from
Jul 20, 2022
Merged

feat: add typesafe client side env variables #209

merged 14 commits into from
Jul 20, 2022

Conversation

nramkissoon
Copy link
Contributor

@nramkissoon nramkissoon commented Jul 18, 2022

Add typesafe client side env variables

  • I reviewed linter warnings + errors, resolved formatting, types and other issues related to my work
  • The PR title follows the convention we established conventional-commit
  • I performed a functional test on my final commit

Implements #195

Separates client and server env schemas. Also added an additional check to ensure client env variable names start with NEXT_PUBLIC_.

@nramkissoon nramkissoon changed the title Add typesafe client side env variables feat: add typesafe client side env variables Jul 18, 2022
@juliusmarminge
Copy link
Member

I would kind of like them to be part of the same env schema when they reach the developer. Example:

// client envschema
// define and export env schema
// server envschema
// define and export env schema
// env validator
// validate client
// validate server 
// merge and export validated env-vars
// usage
import { env } from "envpath";

const pusherKey = env.NEXT_PUBLIC_PUSHER_KEY; // begins with next_public so we know its client

const dbUrl = env.DATABASE_URL; // doesn't begin with next_public so we know its server

Or would this be confusing that you write the schemas in two separate files but import only a single?

@nramkissoon
Copy link
Contributor Author

I would kind of like them to be part of the same env schema when they reach the developer. Example:

// client envschema
// define and export env schema
// server envschema
// define and export env schema
// env validator
// validate client
// validate server 
// merge and export validated env-vars
// usage
import { env } from "envpath";

const pusherKey = env.NEXT_PUBLIC_PUSHER_KEY; // begins with next_public so we know its client

const dbUrl = env.DATABASE_URL; // doesn't begin with next_public so we know its server

Or would this be confusing that you write the schemas in two separate files but import only a single?

I think merging the schemas makes sense for importing env vars server-side because both public and server-only should be available in that case. It would be cumbersome to have to import 2 different env files in a API route file, for example. I can make the changes to do this.

As for client only env vars, turns out you cannot destructure or dynamically access properties in process.env client side, so this validation does not work client side.

const _clientEnv = clientEnvSchema.safeParse(process.env);
export const env = _clientEnv
//usage
import {env} from "clientEnvPath"

// process.env is empty

We can still use this validation to verify env vars at build time and server-side along with the server env vars however, so no concerns there.

The user will either have to resort to accessing client-side env vars explicitly in process.env like process.env.NEXT_PUBLIC_foo or define and export an env object alongside the schema in order to get autocompletion like so:

// client-env-schema.mjs
import { z } from "zod";

export const envSchema = z.object({
  // Specify your environment variables schema here
  // Be sure to name your environment variables with the prefix "NEXT_PUBLIC_"
  NEXT_PUBLIC_foo: z.string(),
});
// client-env.mjs
import {envSchema} from 'client-env-schema.mjs'
export const env = envSchema.parse({
  NEXT_PUBLIC_DDDD: process.env.DDDD
})
//usage
import {env} from 'client-env.mjs'
...

I'd like some thoughts on which approach is better here.

@juliusmarminge
Copy link
Member

My main concern is we will have 4 different files and it will be confusing what goes where, as well as there being 4 files + the actual .env seems kinda "bloated"?

@nramkissoon
Copy link
Contributor Author

My main concern is we will have 4 different files and it will be confusing what goes where, as well as there being 4 files + the actual .env seems kinda "bloated"?

Spent some time playing with different approaches.

I could not find a way to validate both server and client variables in the same file while being able to import client-side.
Server variables don't show up in the browser so validation naturally breaks there. Wrapping the server var validation in a
if (typeof window === "undefined") will work on the client, but messes up the types on the server such that server vars will appear as string | undefined.

Instead, we can put both schemas in the same file so that the user only needs to edit a single file and reduce any confusion. Then, have a client-env.mjs and a server-env.mjs for validating and importing client and server env vars, respectively. server-env.mjs would have the merged client and server env vars.

So it would be 3 files under an env directory.

@juliusmarminge
Copy link
Member

Spent some time playing with different approaches.

I could not find a way to validate both server and client variables in the same file while being able to import client-side. Server variables don't show up in the browser so validation naturally breaks there. Wrapping the server var validation in a if (typeof window === "undefined") will work on the client, but messes up the types on the server such that server vars will appear as string | undefined.

Instead, we can put both schemas in the same file so that the user only needs to edit a single file and reduce any confusion. Then, have a client-env.mjs and a server-env.mjs for validating and importing client and server env vars, respectively. server-env.mjs would have the merged client and server env vars.

So it would be 3 files under an env directory.

Yep. Came to the same conclusion. Let's see how that looks

@juliusmarminge
Copy link
Member

Hotfixing some tRPC stuff so my bad for the format errors and stuff. Sorry, wanted to get the fix out asap

template/base/src/env/server-env.mjs Outdated Show resolved Hide resolved
template/base/src/env/server-env.mjs Outdated Show resolved Hide resolved
template/base/src/env/server-env.mjs Outdated Show resolved Hide resolved
template/addons/env/env-auth.mjs Show resolved Hide resolved
}

for (let key of Object.keys(_clientEnv.data)) {
if (!key.startsWith("NEXT_PUBLIC_")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the "reverse" validation for server so that you can't "accidentally" expose secrets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, also prevents typos and having to waste time figuring out why your public env vars aren't showing up in the browser.

@nexxeln
Copy link
Member

nexxeln commented Jul 20, 2022

Is this ready to be merged now?

@nramkissoon
Copy link
Contributor Author

Is this ready to be merged now?

I think so. @juliusmarminge I've updated my changes to include what you have in your example https://github.com/juliusmarminge/trpc-rqv3

Copy link
Member

@juliusmarminge juliusmarminge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for your patience :)

juliusmarminge added a commit that referenced this pull request Aug 16, 2022
* chore(docs): updated readme to contain @next

* refactor: inject curried `runPkgManagerInstall` to package installers (#202)

* refactor: export curryable `runPkgManagerInstallOptions`

* refactor: inject curried `runPkgManagerInstall` to installers

* refactor: use curried function in installers

* fix: refactored calls that should have passed `devMode` flag

* fix: typo

* feat: migrate env + next config to esm (#205)

* chore: move lintstaged config into file (#217)

* fix: move gitignore rename call (#215)

* feat: add trpc inference helpers

* fix(#218): lock react-query to v3

* chore(release): 5.1.0

* fix: add missing trpc import

* chore(release): 5.1.1

* fix: handle pkg with version in noInstall mode (#220)

* fix: typo in readme

* feat: add db.sqlite-journal to gitignore (#221)

Co-authored-by: Shoubhit Dash <[email protected]>

* fix: typo - rename 'varibles' to 'variables' (#223)

* fix: missing DATABASE_URL (#222)

* chore(release): 5.2.0

* fix: update url in auth prisma schema

* chore(release): 5.2.1

* feat: add typesafe client side env variables (#209)

* feat: add typesafe client side env variables

* refactor: clean up type-safe client side env vars

* fix: add inferProcedureInput/Output imports in trpc utils

* refactor: client env validation

* refactor: run prettier

* refactor: clean up files and formatter func

* fix: validate public env vars are not in server schema

* fix: add ts-check to mjs files

Co-authored-by: Shoubhit Dash <[email protected]>

* fix: title being offset when using yarn/pnpm

* chore(release): bump beta version

* chore: bump Next.js to 12.2.3

* chore(release): bump beta version

* feat: improved logo typography (#238)

* chore: bump beta version

* fix: remove semicolon in _app.tsx with next-auth to avoid early return

* chore: bump beta version

* chore: add protected trpc router example (#194)

* chore: add link to community (#229)

* chore: add link to community info

* chore: add discord badge

* fix: t3 discord id

* fix: discord link on badge

Co-authored-by: Shoubhit Dash <[email protected]>

* docs(readme): add usage for beta and next version (#243)

* chore: add js files to format script (#244)

* chore: remove credentials provider (#246)

* refactor(cli): add module path aliasing (#247)

* feat: updating tailwind and postcss config's to use .cjs (#242)

Co-authored-by: Shoubhit Dash <[email protected]>
Co-authored-by: Julius Marminge <[email protected]>

* fix: clarify some comments and rename some files in env (#245)

* fix: clarify some comments and rename some files in env

* fix: filenames in installer

* fix: import in next.config

* fix: import after filechange

* fix: forgotten rename

Co-authored-by: Shoubhit Dash <[email protected]>

* chore(release): bump beta version

* fix: added JSDoc type to clientEnv in all env-schema (#240)

* chore(release): 5.3.0

* refactor(utils): remove unnecessary block (#252)

* chore: update favicon (#253)

* fix: missing next-auth types (#255)

fixes #254

* fix: height on small screens (#256)

* docs: expand t3-app README (#248)

* revert: extend regular next-auth module (#257)

* revert: extend regular next-auth module

* refactor: move typedefs to `src/types`

Co-authored-by: Shoubhit Dash <[email protected]>

* feat: add deployment strategy to generated README (#258)

* fix(docs): remove extra s in link

* feat: add deployment strategy to readme

* docs: add instructions on building docker image to readme (#265)

* Add instructions on building docker image

Following the same method given in NextJs examples

* fix: prettier check

run prettier format to fix formatting issues

* docs: small adjustments (#266)

* docs: add instructions on building docker image to readme (#265)

* Add instructions on building docker image

Following the same method given in NextJs examples

* fix: prettier check

run prettier format to fix formatting issues

* docs: small adjustments

* feat: set appName to directory on 'npx create-t3-app .' (#273)

* fix(prisma): make db changes for next-auth on mysql obvious (#275)

* chore(release): 5.4.0

* feat: make session.user.id non-nullable (#282)

* chore(release): 5.5.0

* docs: update tRPC v10 docs link (#291)

* fix: change ! reversion to equality in env validation (#293)

* feat: Include ct3a version as metadata in generated app (#305)

* fix: parse path returned from process.cwd() before using it (#303)

Co-authored-by: Shoubhit Dash <[email protected]>
Co-authored-by: Julius Marminge <[email protected]>

* chore(release): 5.6.0

* chore: upload T3 logo to github (#311)

* chore: upload T3 logo to github

* chore: format

* chore: ignore package-lock (#315)

* chore: Publish Workflow and other Github enhancements (#308)

* chore: switch standard-version to changeset

* chore: add version workflow

* chore: issue forms and pr labeler

* chore: prettier

* chore: add missing credit

* chore: adjust some commands

* chore: update contributing guidelines

* chore: prettier

* chore: adjustments

* feat: add minimal ts-linter

* fix: lint warnings

* revert eslinter

* chore(ci): dont use reserved word `package`

* chore(ci): require -> import

* chore(ci): authenticate to npm

* chore: update pr-template

* chore(ci): next-release

* chore(ci): release workflow

* chore(ci): update scrips

* chore(ci): update release versioning

* chore(ci): update versioning scripts

* chore(ci): fix no-bin error

* feat: add base tslint (#317)

* chore(app): version bumps

* feat: add base linter for ts

* fix: test ignoring base

* fix: test setting root

* fix: test add parser

* fix: test disabling exclude

* chore: changeset

* chore(ci): test changeset with npx

* chore(ci): test npm

* chore(ci): give proper commit message and title

* chore(release): remove standard-version msg from changelog

* chore(release): version packages (#319)

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* chore(release): public access

* refactor: turborepo with cli and www (#320)

* docs: add docs site built with Astro (#300)

* docs: add docs site built with Astro

* docs: prettier

Co-authored-by: Shoubhit Dash <[email protected]>

* refactor: make monorepo

* chore(fix): tailwindcss/typography peer deps

* refactor: docusaurus migration

* docs: styling

* fix: linting

* fix: typecheck

* fix: no typecheck www

* fix: restore tsconfig

* fix(ci): cache all node_modules

* fix: more caching

* fix: missing lint plugin

* fix(ci): cwd

* fix(ci): path

* docs: start on collection

* docs: headers

Co-authored-by: Kroucher <[email protected]>
Co-authored-by: Shoubhit Dash <[email protected]>
Co-authored-by: sir-mr-bean <[email protected]>

* chore: set type module

* fix: typo in pub-script

* fix: create npmrc in cli pkg

* fix: path in version script

* refactor: turborepo next

Co-authored-by: Oscar Chic <[email protected]>
Co-authored-by: Daniel Roe <[email protected]>
Co-authored-by: JLN13X <[email protected]>
Co-authored-by: Hauke Schnau <[email protected]>
Co-authored-by: Azmi Makarima <[email protected]>
Co-authored-by: Christopher Ehrlich <[email protected]>
Co-authored-by: Shoubhit Dash <[email protected]>
Co-authored-by: Nicholas Ramkissoon <[email protected]>
Co-authored-by: Vikrant Bhat <[email protected]>
Co-authored-by: Guillermo Antony Cava Nuñez <[email protected]>
Co-authored-by: Abui <[email protected]>
Co-authored-by: John Daly <[email protected]>
Co-authored-by: Simon Green Kristensen <[email protected]>
Co-authored-by: XLS <[email protected]>
Co-authored-by: Omar López <[email protected]>
Co-authored-by: Krish <[email protected]>
Co-authored-by: cyremur <[email protected]>
Co-authored-by: Samuel Gunter <[email protected]>
Co-authored-by: Khalil Omer <[email protected]>
Co-authored-by: BWsix <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kroucher <[email protected]>
Co-authored-by: sir-mr-bean <[email protected]>
devvianto605 pushed a commit to devvianto605/create-devviantex-nextjs-app-deprecated that referenced this pull request Jun 9, 2024
* feat: add typesafe client side env variables

* refactor: clean up type-safe client side env vars

* fix: add inferProcedureInput/Output imports in trpc utils

* refactor: client env validation

* refactor: run prettier

* refactor: clean up files and formatter func

* fix: validate public env vars are not in server schema

* fix: add ts-check to mjs files

Co-authored-by: Shoubhit Dash <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants