Repository navigation
Backport ten fixes to 20.1.x for 20.1.1 - #3798
Conversation
…angular#3738) The SSR deploy builder shelled out with values read straight from angular.json: the package manager and each server externalDependencies entry reached `execSync` as a command string, as did the functions output path. A malicious or cloned workspace could run arbitrary commands the moment a developer ran `ng deploy`. Both calls now go through a single runner built on cross-spawn, which passes arguments as argv entries and launches the Windows .cmd shims that child_process.execFile cannot, so the deploy keeps working cross-platform without a shell. The package manager is checked against a supported set and dependency names are rejected unless they are plain specifiers. Tests cover the validators and assert the call sites route through the shell-free runner, so reverting to a shell or dropping a validator fails the suite. (cherry picked from commit 0e8ed0c)
…ring (angular#3726) spawnAsync built each gcloud command as one string and split it on whitespace before calling spawn(), so any deploy option containing a space was chopped into extra argv entries and a value from angular.json could add flags of its own. It now takes command and args separately and the three call sites pass arrays, removing the join and split entirely. The close handler also rejected only on exit code 1, so a gcloud failure with any other code, or a process killed by a signal, resolved as success and a failed deploy reported as done. It now rejects on any non-zero or null code. The Cloud Run argument construction moved into two exported functions so tests can assert the argv shape without mocking spawn, and functionName and region gained schema patterns, since both also reach the generated Cloud Functions source. (cherry picked from commit 778625a)
…gular#3274) Running ng deploy from Windows with the Cloud Run SSR option wrote the generated run/package.json entry paths with backslashes, which the Linux-based Cloud Run runtime cannot resolve. The manifest path is now built with forward slashes explicitly, since the target runtime is Linux regardless of the deploying machine. Fixes angular#3098 Co-authored-by: Armando Navarro <adnavarro@google.com> (cherry picked from commit 73acea7)
…es (angular#3739) The SSR deploy builders interpolate several angular.json values into generated artifacts that are later executed: a server build target's outputPath into the Cloud Function index.js and the Cloud Run package.json start script, functionName into the exports assignment, region into the .region() call, and functionsNodeVersion into the Cloud Run Dockerfile FROM line. region is escaped structurally with JSON.stringify in the template, and the start script now quotes its path, so a shell no longer splits or expands it. On top of that, outputPath, functionName and functionsNodeVersion are screened before code generation (assertSafeOutputPath, assertSafeFunctionName, assertSafeNodeVersion). assertSafeOutputPath rejects only what is still live once the start script is quoted: quotes, a backslash and line terminators, which break out of the require() string literal, `$` and a backtick, which are still command substitution inside double quotes, and a leading dash, which node reads as a flag. functionName is only screened on the Functions path, where it becomes a JavaScript identifier. functionsNodeVersion is screened against Docker's tag grammar, since the value is a node image tag, with latest excluded because its slim variant is published as node:slim. The functionName and region schema patterns are not repeated here. Stricter versions of both already landed on main in angular#3726. The functionsNodeVersion schema pattern stays here and matches the runtime check exactly. The TODO above the gcloud calls is restored in narrowed form, covering the values that are still unvalidated: firebaseProject, vpcConnector, and the outputPath deploy option. (cherry picked from commit fb6796b)
…ngular#3769) firebase-tools 15.26 runs non-interactively when it detects an AI agent or when stdin is not a terminal, and in that mode login() returns undefined instead of the signed-in account. `ng add` and `ng deploy` crashed reading `.email` from it. getActiveAccount passes `interactive: true`, which returns the account without a prompt once one is signed in. The `firebase login` and `firebase login:add` commands setup starts get `--interactive` for the same reason. Setup no longer passes its options to login(), since they carry the Angular project name, which firebase-tools rejects as a project id for names like `myApp`. The Quickstart stops telling readers to install firebase-tools 14, and says what a stopped run leaves behind. Fixes angular#3768 The Quickstart changes are left out, since docs are not backported. (cherry picked from commit 38dfc3f)
…ngular#3784) Every canary publish moved the `canary` dist-tag, whatever commit it was built from. Two merges close together could publish out of order, and a re-run of an older run could publish its build last. The publish job now clones the repository's commit history and publishes a canary only when its commit comes after the commit of the canary on npm, or is the same commit under a new version. A build of an earlier commit is skipped with a warning. Commits are compared rather than versions, because a version can be higher for an older commit. Canary publishes also run one at a time, queued, so each check reads what the previous publish left on npm. Release publishes are unchanged. A scheduled run on an unchanged main now skips with a notice instead of failing on the duplicate version. Fixes angular#3783 (cherry picked from commit eee3486)
…gular#3792) npm 10, and npm 11 before 11.20, resolve a missing optional peer anyway while installing. For an app whose package.json uses caret ranges and whose Angular is one patch behind the newest, npm picks the newest @angular/platform-server, which requires that exact newest @angular/core, and the install stops with ERESOLVE. Every current Node line bundles one of those npm versions. Nothing in the published package imports @angular/platform-server, so the peer is removed. Apps that render on the server still install it themselves, as they already do. (cherry picked from commit bbce8f0)
… after it (angular#3794) * fix(ci): choose a release's dist-tag when it publishes, and move next after it A stable tag published to latest unconditionally, so a 20.x release tagged after 21.0.0 would move latest back to 20.x. A stable release now goes to latest only when it ranks above npm's latest and every other stable git tag, since a higher release can still be in npm's publish-time malware scan. Otherwise it goes to v<major>-lts, and the job fails if it does not rank above that tag or another release of its major. Every publish now waits until npm lists it, in one npm-publish queue, so the next publish reads the real dist-tags. That stops a queued canary from reading a stale canary tag. After a release reaches latest, next moves up to it, and a failure there prints the npm dist-tag command that finishes it. A re-run of a published release skips npm publish and goes on to the wait and the next move. The steps live in tools/publish-job.js and tools/release-tag.js so specs cover them, and build.sh no longer picks a tag. * refactor(ci): move the canary check into tools/publish-job.js The check that skips a canary of an earlier commit keeps the same decisions and messages, and its specs now cover each branch. It runs only for pushes and scheduled runs, and a failed read of npm's dist-tags now says why. (cherry picked from commit 31fb2ae)
The publish job's specs (tools/publish-job.jasmine.ts and tools/release-tag.jasmine.ts) live under tools/. On main, angular#3780 added this include, and angular#3780 itself stays off 20.1.x, so without it build:jasmine never compiles those 39 specs and test:node skips them without failing.
…ar#3796) fromTask declared no return type, so the published typings import whatever path the typings bundler picks for the inferred firebase type. In 20.x that is 'firebase/compat', which firebase's exports map does not list. Apps using moduleResolution "bundler", the setting the @angular/build migration moves apps to, fail with TS2307 in @angular/fire/compat/storage. 21.0.0-rc.1 happens to emit 'firebase/compat/app' and compiles, but ng-packagr 22.2's new typings bundler cannot parse the inferred type and fails the library build. Declaring Observable<UploadTaskSnapshot> removes the inferred import. The type is unchanged: UploadTaskSnapshot is the compat alias for firebase.storage.UploadTaskSnapshot. Also drop a comment explaining a firebase import that angular#3421 removed in 2023 as unused. The import only existed to steer these typings. Fixes angular#3677 (cherry picked from commit 4c5ba7e)
tyler-reitz
left a comment
There was a problem hiding this comment.
Approved.
I checked each pick against its source on main. Every deviation is one of the conflict resolutions you listed, and two picks are byte-identical. 166 specs, 0 failures reproduces locally, and the specs do gate: neutering releaseTag gives 9 failures, neutering assertSafeOutputPath gives 15. The built package drops only the platform-server peer and emits no publish.sh. Loading the built actions.js resolves cross-spawn zero times externally, so the bundling claim holds. Running release-tag.js against npm's current dist-tags and all 144 git tags puts 20.1.1 on latest and 21.0.0-rc.2 on next.
Two questions, neither blocking:
- #3793 and #3770 merged to
mainin the same window and are not in this set. Deliberate, or worth a follow-up backport? - The new
functionNameandfunctionsNodeVersionschema patterns reject values 20.1.0 accepted, such as a name starting with a digit, or>=18. Fine by me, but it deserves a line in the 20.1.1 notes.
Agreed on Rebase and merge. The squash we normally use would collapse the nine into one commit on 20.1.x.
…ngular#3770) AngularFire wrapped beforeAuthStateChanged so that registering the hook added a pending task, cleared only when the callback first runs. Firebase runs that callback only on a sign-in or sign-out, so for a visitor who does neither the app never became stable. Registered on the server, it failed ng build during route extraction and left server-rendered requests without a response. This restores the blockUntilFirst: false override from angular#3590, which angular#3613 dropped without comment while adding log-level overrides next to it. The callback still runs inside Angular's zone and injection context, and its returned promise still reaches Firebase, so a rejection still cancels the sign-in. A call outside an injection context now logs its per-call warning only at the verbose level, as onMessage does. Fixes angular#3748 docs(auth): scope the beforeAuthStateChanged note to rc.1 and earlier Merging this change closes angular#3748, so the section's present-tense note would point at a closed issue. Also removed the false claim that the @angular/fire/auth import makes ng build hang: the guide registers the hook only in the browser, so its own build succeeds. The docs/auth.md change is left out, since it edits a section of the guide that 20.1.x does not have. (cherry picked from commit f182972)
|
Thanks for checking every pick against
|
This brings ten fixes from
mainto20.1.xfor the 20.1.1 release. Each commit is agit cherry-pick -xof the squash commit onmain, in the order they merged there, plus one config line. Please merge with Rebase and merge, so each fix stays its own commit on20.1.x.Fixes for v20 users
beforeAuthStateChangedfrom@angular/fire/authkeeps the app from ever becoming stable for a visitor who never signs in or out (beforeAuthStateChangedimported from@angular/fire/authmakesng buildfail during route extraction #3748).ng addstops withInvalid project id: myApp.for a project named with capital letters, and crashes on firebase-tools 15.26+ when it is not attached to a terminal.@angular/fire/compat/storagetypings importfirebase/compat, which fails withTS2307undermoduleResolution: "bundler"(@angular/build: TS2307: Cannot find module 'firebase/compat' #3677).npm installfails withERESOLVEon an app one Angular patch behind, because of the unused optional@angular/platform-serverpeer (@angular/cli v20. NPM cannot resolve @angular/fire #3667).ng deploybuilder:gcloudstep is ignored, and the hosting deploy still runs.angular.jsonvalues reach a shell and the generated function code without escaping.Release tooling
latestonly while it ranks above every other release, and tov20-ltsotherwise, so a 20.x release tagged after 21.0.0 cannot movelatestback. fix(ci): keep the canary dist-tag from moving back to an older build #3784 is picked first so fix(ci): choose a release's dist-tag when it publishes, and move next after it #3794 applies cleanly.tsconfig.jasmine.jsongets thetools/**/*.jasmine.tsinclude from fix(build): name canaries above every published release of their major #3780, so fix(ci): choose a release's dist-tag when it publishes, and move next after it #3794's specs run on this branch.Conflicts
All conflicts are import lines, version ranges or lines that exist only on one branch:
fs-extraimport, and added@types/cross-spawnand thecross-spawnimport../actionsimport specifier.getActiveAccounthelper tofirebaseTools.ts, and left the docs as they are on this branch.@angular/platform-serverpeer.tools/build.shkeeps this branch's canary version line, sincecanary-version.js(fix(build): name canaries above every published release of their major #3780) is not here, and drops theNPM_TAGandpublish.shlines the new publish job replaces.docs/auth.md, since its docs change edits a section of the guide that 20.1.x does not have.Verification
./tools/build.sh,build:jasmineandtest:nodepass with 166 specs and 0 failures, up from 29 on20.1.x. The Chrome suite gives the same result as on20.1.x(108 passed, 48 skipped). The built schematics bundlecross-spawn, so the published package needs no new dependency, and the only manifest change is the removed peer.