feat(nextjs): Default to VERCEL_ENV as environment#7227
Conversation
packages/nextjs/src/client/index.ts
Outdated
|
|
||
| options.environment = | ||
| options.environment || | ||
| (process.env.NEXT_PUBLIC_VERCEL_ENV ? `vercel-${process.env.NEXT_PUBLIC_VERCEL_ENV}` : undefined) || |
There was a problem hiding this comment.
l: Can we maybe put this into a small util like:
function getVercelEnv(): string | undefined {
const vercelEnv = process.env.NEXT_PUBLIC_VERCEL_ENV;
return vercelEnv && `vercel-${vercelEnv}`;
}Feels better to me IMHO than to sprinkle this through the rest of the code - then we can just do e.g.
options.environment = options.environment || getVercelEnv() || process.env.NODE_ENV;There was a problem hiding this comment.
It would just deduplicate this once because we have VERCEL_ENV for node and edge and NEXT_PUBLIC_VERCEL_ENV for the client.
There was a problem hiding this comment.
can't we just dynamically chose via a boolean?
I would also like to see this as a func because this is totally something that where we only update two, but forgot to update one (because of merge conflicts, quick fix PR etc.)
size-limit report 📦
|
Replay SDK metrics 🚀
develop |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Revision | LCP | CLS | CPU | JS heap avg | JS heap max | netTx | netRx | netCount | netTime |
|---|---|---|---|---|---|---|---|---|---|
| 7f4c4ec | +56.64 ms | -0.00 ms | +5.57 pp | +927.42 kB | +1.06 MB | +2.21 kB | +41 B | +1 | +110.83 ms |
| 00d2360 | +55.18 ms | +0.00 ms | +2.23 pp | +934.14 kB | +1.05 MB | +2.22 kB | +41 B | +1 | +71.65 ms |
Last updated: Mon, 20 Feb 2023 09:46:38 GMT
|
@lforst I'm not sure I fully understand the reason behind the prefix. I'm also concerned that it will now mark new production events as BTW, you can just use |
I say the prefix is useful to discriminate between the vercel environment a build gets deployed to and the local environment where it is likely that People will have the option to override this default by setting the environment option. |
Fixes #6993
This PR is just a quick win that should improve the ootb experience for Vercel on Next.js by setting the environment option to a sensible default provided by the hosting provider.
The
vercel-prefix is so you can discern the verceldevelopmentenvironment from local development because there theNODE_ENVvar withdevelopmentis likely gonna be picked up.