-
Notifications
You must be signed in to change notification settings - Fork 30.4k
Warn when inlining Vercel system env vars #89304
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
base: canary
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
afca7ae to
0d703e5
Compare
d9364bb to
e4048ef
Compare
packages/next/src/lib/static-env.ts
Outdated
| process.env.NEXT_PUBLIC_VERCEL = process.env.NEXT_PUBLIC_VERCEL || '' | ||
| process.env.NEXT_PUBLIC_CI = process.env.NEXT_PUBLIC_CI || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL = | ||
| process.env.NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_REGION = | ||
| process.env.NEXT_PUBLIC_VERCEL_REGION || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_SKEW_PROTECTION_ENABLED = | ||
| process.env.NEXT_PUBLIC_VERCEL_SKEW_PROTECTION_ENABLED || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_AUTOMATION_BYPASS_SECRET = | ||
| process.env.NEXT_PUBLIC_VERCEL_AUTOMATION_BYPASS_SECRET || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_PROJECT_ID = | ||
| process.env.NEXT_PUBLIC_VERCEL_PROJECT_ID || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PROVIDER = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PROVIDER || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_SLUG = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_SLUG || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_OWNER = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_OWNER || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_ID = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_REPO_ID || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_ENV = process.env.NEXT_PUBLIC_VERCEL_ENV || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_TARGET_ENV = | ||
| process.env.NEXT_PUBLIC_VERCEL_TARGET_ENV || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL = | ||
| process.env.NEXT_PUBLIC_VERCEL_BRANCH_URL || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_REF = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_REF || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PULL_REQUEST_ID = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PULL_REQUEST_ID || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_URL = process.env.NEXT_PUBLIC_VERCEL_URL || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_DEPLOYMENT_ID = | ||
| process.env.NEXT_PUBLIC_VERCEL_DEPLOYMENT_ID || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_OIDC_TOKEN = | ||
| process.env.NEXT_PUBLIC_VERCEL_OIDC_TOKEN || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_SHA || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_MESSAGE = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_MESSAGE || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_AUTHOR_LOGIN = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_AUTHOR_LOGIN || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_AUTHOR_NAME = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_COMMIT_AUTHOR_NAME || '' | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PREVIOUS_SHA = | ||
| process.env.NEXT_PUBLIC_VERCEL_GIT_PREVIOUS_SHA || '' |
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 do we need to init these environment variables eagerly? if they don't exist then i guess they don't get replaced rather than an error?
leave a comment to that effect
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.
just for local testing for now, not sure yet if we are going to keep this
because these env vars are only when deploying, and not locally, which makes the DX worse
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.
yeah that makes sense
if we keep it, maybe consider a constant list of a strings in a loop?
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 wrong to inline them with an empty string though
so i'm thinking about doing FreeVarReference::Warning { inner: None } instead to support warning but not replacing.
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.
With some more magic, it's now gated behind NOW_BUILDER like all other Vercel-specific stuff
But ideally, it would be printed in dev as well. Should we just always do this unconditionally? These are Vercel-specific env vars anyway?
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.
yeah i think doing it all the time is reasonable
Failing test suitesCommit: cbc6ad7 | About building and testing Next.js
Expand output● app dir - prefetching › should show layout eagerly when prefetched with loading one level down ● app dir - prefetching › should immediately render the loading state for a dynamic segment when fetched from higher up in the tree |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **434 kB** → **434 kB** ✅ -2 B81 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
|
e4048ef to
e11a725
Compare
7dde8d6 to
8db23fd
Compare
e11a725 to
8aa869f
Compare
8db23fd to
dee9834
Compare
8aa869f to
cee941c
Compare
dee9834 to
243fda5
Compare
cee941c to
0a54adb
Compare
243fda5 to
2e3fb9b
Compare
0a54adb to
9b0b1cb
Compare
9b0b1cb to
cbc6ad7
Compare

Part of PACK-6641
We don't have a way to silence (ignore-comment) warnings though
And additionally, warnings in node_modules are always hidden...