Skip to content

fix(cloudformation-diff): DependsOn changes are invisible for lists of two or more - #1825

Open
Adityaj0 wants to merge 1 commit into
aws:mainfrom
Adityaj0:fix/diff-dependson-multi-element
Open

fix(cloudformation-diff): DependsOn changes are invisible for lists of two or more#1825
Adityaj0 wants to merge 1 commit into
aws:mainfrom
Adityaj0:fix/diff-dependson-multi-element

Conversation

@Adityaj0

Copy link
Copy Markdown

Fixes #1822

Reason for this change

dependsOnEqual compares two DependsOn arrays "irrespective of element order", but the inner loop ends in an unconditional break, so it only ever runs for j = 0. Its single return false is guarded by j === lvalue.length - 1, which on that one iteration is only true for a one-element array. Any two same-length arrays with two or more entries therefore compare as equal, whatever they contain — the length check is the only comparison that survives.

The user-visible effect: cdk diff reports no change for a resource whose dependency list was rewritten, as long as the entry count stays the same. CDK emits multi-element DependsOn routinely (a single lambda.Function produces two entries, as #1602 noted), so this is a realistic shape.

Description of changes

Replaced the loop with a multiplicity-preserving match: each element on the left must find a distinct, not-yet-consumed element on the right. That is the order-insensitive comparison the original comment describes, and it also handles duplicates correctly (['A','A'] vs ['A','B'] is now unequal).

Neither input array is reordered — only a local array of indices is mutated — so the no-mutation property established by #1575 / #1602 is preserved. There is an explicit test for that.

The three behaviours the suite already covered are unchanged: reordering is equal, differing lengths are unequal, and 'A' vs ['A'] is equal (handled by the branch above this one).

This surfaces changes that cdk diff currently hides. It does not invent differences: every newly reported change is a real edit to the template.

Description of how you validated changes

  • Five new tests in template-and-changeset-diff-merger.test.ts, next to the existing DependsOn cases: four parameterised cases for same-length-but-different arrays (2 elements fully replaced, 2 elements partially replaced, 3 elements, and a duplicate-sensitivity case), plus one asserting reorder-equality with no mutation of the caller's arrays.
  • All four of the new "unequal" cases fail on main and pass with this change — confirmed by stashing the lib change and re-running.
  • Full package suite: 151 tests passing.

Checklist

  • This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
    • Release notes for the new version:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

…f two or more

`dependsOnEqual` intends to compare two DependsOn arrays irrespective of
element order, but the inner loop ends in an unconditional `break`, so it
only ever runs for j = 0. The `return false` is guarded by
`j === lvalue.length - 1`, which for any array longer than one element is
never true on that single iteration. Two same-length arrays therefore
always compare as equal, whatever they contain.

The effect is that `cdk diff` reports no change for a resource whose
DependsOn list is completely rewritten, as long as the number of entries
stays the same.

Replace the loop with a multiplicity-preserving match of each element on
the left against a distinct element on the right. Neither input array is
reordered, keeping the property that aws#1602 established.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cloudformation-diff: DependsOn changes are invisible in cdk diff for lists of two or more entries

1 participant