Skip to content

fix: add missing build-time env vars#7063

Open
hrishikesh-k wants to merge 19 commits into
mainfrom
hk/build-env
Open

fix: add missing build-time env vars#7063
hrishikesh-k wants to merge 19 commits into
mainfrom
hk/build-env

Conversation

@hrishikesh-k

Copy link
Copy Markdown
Contributor

Currently, when building with CLI without logging into ay account, the following variables get populated:

{
  DEPLOY_ID: { sources: [ 'general' ], value: '0' },
  BUILD_ID: { sources: [ 'general' ], value: '0' },
  DEPLOY_PRIME_URL: {
    sources: [ 'general' ],
    value: 'https://<branch>--site-name.netlify.app'
  },
  DEPLOY_URL: { sources: [ 'general' ], value: 'https://0--site-name.netlify.app' },
  CONTEXT: { sources: [ 'general' ], value: 'production' },
  NETLIFY_LOCAL: { sources: [ 'general' ], value: 'true' },
  BRANCH: { sources: [ 'general' ], value: '<value>' },
  HEAD: { sources: [ 'general' ], value: '<value>' },
  COMMIT_REF: {
    sources: [ 'general' ],
    value: '<value>'
  },
  CACHED_COMMIT_REF: {
    sources: [ 'general' ],
    value: '<value>'
  },
  PULL_REQUEST: { sources: [ 'general' ], value: 'false' },
  LANG: { sources: [ 'general' ], value: 'en_US.UTF-8' },
  LANGUAGE: { sources: [ 'general' ], value: 'en_US:en' },
  LC_ALL: { sources: [ 'general' ], value: 'en_US.UTF-8' },
  GATSBY_TELEMETRY_DISABLED: { sources: [ 'general' ], value: '1' },
  NEXT_TELEMETRY_DISABLED: { sources: [ 'general' ], value: '1' },
  NETLIFY_CLI_VERSION: { sources: [ 'internal' ], value: '26.0.1' }
}

Notice how we're missing some other variables like:

SITE_ID
SITE_NAME
URL

This PR adds those variables. Also, for the DEPLOY_PRIME_URL, we set the branch as it is. This PR also slugifies the branch name, so that branch like: foo/bar could become foobar.

@hrishikesh-k hrishikesh-k requested a review from a team as a code owner May 14, 2026 13:57
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds the any-ascii runtime dependency; consolidates Netlify default constants at module scope; sets SITE_ID and SITE_NAME via nullish coalescing in getGeneralEnv; adds an internal slugify helper; derives URL from ssl_url with a fallback and builds DEPLOY_PRIME_URL using a slugified branch; expands the read-only env allowlist to protect deployment-related variables; updates many test snapshots and a few tests to reflect the new env fields and normalized DEPLOY_PRIME_URL values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • serhalp
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description provides clear context about the problem (missing env vars), the solution (adding SITE_ID, SITE_NAME, URL), and implementation details (branch slugification). However, it does not reference an issue number as required by the template. Add the issue number reference (Fixes #<issue_number>) at the top of the description to fully comply with the template structure.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding missing build-time environment variables (SITE_ID, SITE_NAME, URL) to the build configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hk/build-env

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

"severity": "info",␊
"unhandled": false,␊
"location": {␊
"buildLogs": "https://app.netlify.com/sites/site-name/deploys/0",␊

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be produced for those "mock" values?

@hrishikesh-k hrishikesh-k May 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think, yes. These lines are coming from here:

const getBuildLogs = function ({ SITE_NAME, DEPLOY_ID }) {
if (SITE_NAME === undefined || DEPLOY_ID === undefined) {
return
}
return `${NETLIFY_ORIGIN}/sites/${SITE_NAME}/deploys/${DEPLOY_ID}`
}
const NETLIFY_ORIGIN = 'https://app.netlify.com'
. Previously, DEPLOY_ID existed, SITE_NAME didn't. Now SITE_NAME also exists resulting in the line existing. Based on the code, I think it makes sense to have these lines printed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am a bit conflicted on this - not sure if one is worse then the other, but that buildLogs url won't ever work, so that's why I was wondering wether we should return undefined for the mocked values case (as in expand conditions that are checked to avoid constructing url if values are mocked)

But I guess same can be same about mocked SITE_NAME and all the others, that they won't work either in practice - so probably matter of degree on what make sense to "mock" and what does not 🤔

Comment thread packages/config/package.json Outdated
Comment thread packages/config/package.json Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants