-
Notifications
You must be signed in to change notification settings - Fork 65
fix: add deploy-related environment variables to local builds #4209
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
Conversation
|
||
const NETLIFY_DEFAULT_DOMAIN = '.netlify.app' | ||
// `site.name` is `undefined` when there is no token or siteId | ||
const DEFAULT_SITE_NAME = 'site-name' |
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.
In local builds, this default value for the site name is used by the DEPLOY_URL
and DEPLOY_PRIME_URL
environment variables, but intentionally not by the SITE_NAME
environment variable. This is to be consistent with SITE_ID
which is also unavailable when the authentication token or siteId
has not been provided.
Please note this situation is an edge case, since it cannot happen in neither production nor CLI builds, only when @netlify/build
is called programmatically (which at the moment is mostly only used in @netlify/build
automated tests).
1918a60
to
6ea1b65
Compare
6ea1b65
to
2f57561
Compare
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.
Thanks @ehmicky, the code looks great.
I added 2 comments on possible edge cases to handle.
} | ||
} | ||
|
||
const NETLIFY_DEFAULT_DOMAIN = '.netlify.app' |
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.
I wonder if NETLIFY_DEFAULT_DOMAIN
is still used when a user has a custom domain configured.
What will be the host for DEPLOY_PRIME_URL
and DEPLOY_URL
in that case?
If not, we might be able to take them from the siteInfo
(if it is available)
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.
Good question. Yes, .netlify.app
is still used even with a custom domain, but only for DEPLOY_URL
and DEPLOY_PRIME_URL
, not for URL
. This is documented as such, but I also confirmed this with a manual test on my test site which uses a custom domain (see build logs).
return { | ||
URL: sslUrl, | ||
REPOSITORY_URL, | ||
DEPLOY_PRIME_URL: `https://${branch}--${name}${NETLIFY_DEFAULT_DOMAIN}`, |
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.
Where do we get branch
and deployId
from?
branch
might not be available if running from a non git
folder
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.
deployId
is taken from:
build/packages/config/src/options/main.js
Line 47 in e118a5c
deployId: combinedEnv.DEPLOY_ID, |
Before this PR, it was set either the @netlify/config
flag deployId
(which we use in production) or the DEPLOY_ID
environment variable. With this PR, it also defaults to 0
for CLI and programmatic builds.
branch
is taken from:
https://github.com/netlify/build/blob/main/packages/config/src/options/branch.js#L3-L9
It has a default value when used in a non-git environment.
Fixes #3904
This PR builds on top of #3931 which was created by @lukasholzer a few months ago to solve this issue.
This adds dummy values for 4 environment variables in local builds (not production builds):
DEPLOY_ID
,BUILD_ID
,DEPLOY_URL
andDEPLOY_PRIME_URL
.This is a rather small change from a production code standpoint. However, it updates a high amount of test snapshots, but those changes are all identical, essentially just adding those new default values to the test snapshots.
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.