Skip to content

fix(docker): guard commitSha null in plugin interpolation#33275

Merged
Coly010 merged 1 commit intonrwl:masterfrom
emiliosheinz:fix/error-when-commit-sha-is-null
Oct 28, 2025
Merged

fix(docker): guard commitSha null in plugin interpolation#33275
Coly010 merged 1 commit intonrwl:masterfrom
emiliosheinz:fix/error-when-commit-sha-is-null

Conversation

@emiliosheinz
Copy link
Copy Markdown
Contributor

Current Behavior

Docker plugin assumed commitSha was always non-null; when null, shortCommitSha.slice caused a runtime error during target interpolation.

Expected Behavior

Plugin should succeed even if latest commit SHA cannot be resolved, simply omitting shortCommitSha-based substitutions.

Changes

  • Added null guard: shortCommitSha now set to commitSha ? commitSha.slice(0,7) : null.
  • Added test "should not throw when commitSha is null" verifying node / target creation succeeds.
  • No breaking changes; only broadens safe input surface.

Additional notes

Logic only executes when commitSha was previously null (error case); normal paths unchanged. If consumers interpolate {shortCommitSha}, they should handle possible null (unchanged if interpolation is already optional).

@emiliosheinz emiliosheinz requested review from a team, Coly010 and jaysoo as code owners October 28, 2025 00:13
@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 28, 2025

👷 Deploy request for nx-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit f9dff93

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 28, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
nx-dev Ready Ready Preview Oct 28, 2025 0:18am

@nx-cloud
Copy link
Copy Markdown
Contributor

nx-cloud Bot commented Oct 28, 2025

View your CI Pipeline Execution ↗ for commit f9dff93

Command Status Duration Result
nx affected --targets=lint,test,test-kt,build,e... ✅ Succeeded 21m 55s View ↗
nx run-many -t check-imports check-lock-files c... ✅ Succeeded 2m 40s View ↗
nx-cloud record -- nx-cloud conformance:check ✅ Succeeded 11s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 2s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded <1s View ↗

☁️ Nx Cloud last updated this comment at 2025-10-28 08:36:10 UTC

@emiliosheinz
Copy link
Copy Markdown
Contributor Author

Hey @Coly010, it would be lovely if we could get this merged and released on a beta channel. It's currently blocking me from moving forward on a personal project 😬

@Coly010 Coly010 merged commit 1089ffc into nrwl:master Oct 28, 2025
15 checks passed
FrozenPandaz pushed a commit that referenced this pull request Oct 28, 2025
## Current Behavior
Docker plugin assumed `commitSha` was always non-null; when `null`,
`shortCommitSha.slice` caused a runtime error during target
interpolation.

## Expected Behavior
Plugin should succeed even if latest commit SHA cannot be resolved,
simply omitting shortCommitSha-based substitutions.

## Changes
- Added null guard: `shortCommitSha` now set to `commitSha ?
commitSha.slice(0,7) : null`.
- Added test "should not throw when commitSha is null" verifying node /
target creation succeeds.
- No breaking changes; only broadens safe input surface.

## Additional notes
Logic only executes when `commitSha` was previously null (error case);
normal paths unchanged. If consumers interpolate `{shortCommitSha}`,
they should handle possible null (unchanged if interpolation is already
optional).

(cherry picked from commit 1089ffc)
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 3, 2025

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants