Skip to content

fix(migrations): Make the safe optional chaining idempotent#68975

Open
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fix/safe-navigation-migration
Open

fix(migrations): Make the safe optional chaining idempotent#68975
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:fix/safe-navigation-migration

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

Our unit tests were missleading, the migration wasn't idempotent and $safeNavigationMigration were added multiple times on consecutive runs.

@angular-robot angular-robot Bot added the area: migrations Issues related to `ng update`/`ng generate` migrations label May 28, 2026
@ngbot ngbot Bot added this to the Backlog milestone May 28, 2026
@JeanMeche JeanMeche added the target: rc This PR is targeted for the next release-candidate label May 28, 2026
@JeanMeche JeanMeche requested a review from atscott May 28, 2026 08:53
// ---------------------------------------------------------------------------

override visitCall(ast: Call, nullSensitive: boolean): any {
if (isSafeNavigationMigrationCall(ast)) {
Copy link
Copy Markdown
Contributor

@atscott atscott May 28, 2026

Choose a reason for hiding this comment

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

is it possible that there would be nested required calls to this? e.g. my?.utility?.service(my?.optional?.argument)? (maybe not the best example but you get the idea)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've added a test that illustrates that we're still good !

Our unit tests were missleading, the migration wasn't idempotent and `$safeNavigationMigration` were added multiple times on consecutive runs.
@JeanMeche JeanMeche force-pushed the fix/safe-navigation-migration branch from d29cada to f579e1a Compare May 28, 2026 16:48
@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker area: migrations Issues related to `ng update`/`ng generate` migrations target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants