Skip to content

Commit

Permalink
feat: jsii-reflect now renders union candidates sorted (#4005)
Browse files Browse the repository at this point in the history
Sorting union members guarantees a uniform order is always presented, making it easier to visually compare assemblies (or pre-process them with `jsii-reflect` before using a textual `diff` tool). This also effects how `jsii-diff` compares assemblies, removing many false positives in breaking change detection.



---

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

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
RomainMuller authored Mar 14, 2023
1 parent 9aef30e commit 1fd6cde
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 24 deletions.
6 changes: 3 additions & 3 deletions packages/jsii-diff/test/classes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ test.each([
{
oldDecl: 'name: string',
newDecl: 'name: string | number',
error: /string \| number is not assignable to string/,
error: /number \| string is not assignable to string/,
},
{
oldDecl: 'name: string | number',
Expand Down Expand Up @@ -413,12 +413,12 @@ test.each([
{
oldDecl: 'name: string',
newDecl: 'name: string | number',
error: /changed to string \| number \(formerly string\)/,
error: /changed to number \| string \(formerly string\)/,
},
{
oldDecl: 'name: string | number',
newDecl: 'name: string',
error: /changed to string \(formerly string \| number\)/,
error: /changed to string \(formerly number \| string\)/,
},
])(
'cannot change a mutable class property type: %p to %p',
Expand Down
4 changes: 2 additions & 2 deletions packages/jsii-diff/test/type-unions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ test('type unions in return structs can be narrowed', () =>

test('type unions in return structs can not be widened', () =>
expectError(
/some of string \| number \| boolean are not assignable to string \| number/,
/some of boolean \| number \| string are not assignable to number \| string/,
`
export interface Henk {
readonly henk: string | number;
Expand Down Expand Up @@ -112,7 +112,7 @@ test('type unions in input structs can be widened', () =>

test('type unions in input structs can not be narrowed', () =>
expectError(
/string \| number is not assignable to string/,
/number \| string is not assignable to string/,
`
export interface Henk {
readonly henk: string | number;
Expand Down
4 changes: 3 additions & 1 deletion packages/jsii-reflect/lib/type-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export class TypeReference {
return `Map<string => ${this.mapOfType.toString()}>`;
}
if (this.unionOfTypes) {
return this.unionOfTypes.map((x) => x.toString()).join(' | ');
const union = this.unionOfTypes.map((x) => x.toString());
union.sort();
return union.join(' | ');
}

throw new Error('Invalid type reference');
Expand Down
18 changes: 9 additions & 9 deletions packages/jsii-reflect/test/__snapshots__/jsii-tree.test.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions packages/jsii-reflect/test/__snapshots__/tree.test.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1fd6cde

Please sign in to comment.