From 60131c6624fa324129a407d110cbe9e2bd03a255 Mon Sep 17 00:00:00 2001 From: Eli White Date: Thu, 19 Feb 2026 23:00:05 -0800 Subject: [PATCH] Fix union type alias sorting (#55603) Summary: TypeAliasTypeAnnotation members in unions were all treated as equal (returning 0) by the sort comparator, which meant the merge-based comparison in compareUnionMemberArrays could not distinguish between different type aliases like ObjectA, ObjectB, and ObjectC. This caused it to compare them positionally instead of by identity, misreporting added/removed union members as type mismatches. The fix compares TypeAliasTypeAnnotation by name, similar to how StringLiteralTypeAnnotation compares by value, allowing the merge algorithm to correctly identify which members were added or removed. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D93262588 --- .../src/SortTypeAnnotations.js | 3 +- .../src/__tests__/ErrorFormattingTests.js | 8 ++ .../src/__tests__/VersionDiffing-test.js | 79 +++++++++++++++++++ .../NativeModule.js.flow | 41 ++++++++++ .../ErrorFormatting-test.js.snap | 32 ++++++++ 5 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 packages/react-native-compatibility-check/src/__tests__/__fixtures__/native-module-with-union-object-added/NativeModule.js.flow diff --git a/packages/react-native-compatibility-check/src/SortTypeAnnotations.js b/packages/react-native-compatibility-check/src/SortTypeAnnotations.js index 537653ff9673..1d984a6f3bd7 100644 --- a/packages/react-native-compatibility-check/src/SortTypeAnnotations.js +++ b/packages/react-native-compatibility-check/src/SortTypeAnnotations.js @@ -153,7 +153,8 @@ export function compareTypeAnnotationForSorting( [originalPositionB, typeB.elementType], ); case 'TypeAliasTypeAnnotation': - return 0; + invariant(typeB.type === 'TypeAliasTypeAnnotation', EQUALITY_MSG); + return typeA.name.localeCompare(typeB.name); case 'MixedTypeAnnotation': return 0; default: diff --git a/packages/react-native-compatibility-check/src/__tests__/ErrorFormattingTests.js b/packages/react-native-compatibility-check/src/__tests__/ErrorFormattingTests.js index a49ff0283ecd..fcf283078b55 100644 --- a/packages/react-native-compatibility-check/src/__tests__/ErrorFormattingTests.js +++ b/packages/react-native-compatibility-check/src/__tests__/ErrorFormattingTests.js @@ -33,6 +33,10 @@ export const okayChanges = [ 'native-component-with-props/NativeComponent', 'native-component-with-props-default-change/NativeComponent', ], + [ + 'native-module-with-union-object/NativeModule', + 'native-module-with-union-object-added/NativeModule', + ], ]; export const incompatibleChanges = [ @@ -130,6 +134,10 @@ export const incompatibleChanges = [ 'native-module-with-union-object/NativeModule', 'native-module-with-union-object-changes/NativeModule', ], + [ + 'native-module-with-union-object-added/NativeModule', + 'native-module-with-union-object/NativeModule', + ], [ 'native-component-with-command/NativeComponent', 'native-component-with-command-changed/NativeComponent', diff --git a/packages/react-native-compatibility-check/src/__tests__/VersionDiffing-test.js b/packages/react-native-compatibility-check/src/__tests__/VersionDiffing-test.js index 1db30cddcf57..aa129e593509 100644 --- a/packages/react-native-compatibility-check/src/__tests__/VersionDiffing-test.js +++ b/packages/react-native-compatibility-check/src/__tests__/VersionDiffing-test.js @@ -118,6 +118,12 @@ const schemaWithUnionFromNative = makeRN( const schemaWithUnionFromNativeChanges = makeRN( 'native-module-with-union-from-native-changes/NativeModule', ); +const schemaWithUnionObject = makeRN( + 'native-module-with-union-object/NativeModule', +); +const schemaWithUnionObjectAdded = makeRN( + 'native-module-with-union-object-added/NativeModule', +); const schemaWithRNNativeComponent = makeRN('native-component/NativeComponent'); const schemaNativeComponentWithCommand = makeRN( @@ -1192,6 +1198,79 @@ describe('buildSchemaDiff', () => { ); }); + it('RN with union object types, and addition', () => { + const schemaDiff = buildSchemaDiff( + schemaWithUnionObjectAdded, // new: ObjectA | ObjectB | ObjectC + schemaWithUnionObject, // old: ObjectA | ObjectB + ); + + expect(summarizeDiffSet(schemaDiff).status).toBe('incompatible'); + expect(schemaDiffExporter(Array.from(schemaDiff)[0])).toEqual( + expect.objectContaining({ + framework: 'ReactNative', + name: 'NativeModule', + status: expect.objectContaining({ + incompatibleSpecs: expect.arrayContaining([ + expect.objectContaining({ + changeInformation: expect.objectContaining({ + incompatibleChanges: expect.arrayContaining([ + expect.objectContaining({ + errorCode: 'addedMemberCases', + }), + ]), + objectTypeChanges: expect.arrayContaining([ + expect.objectContaining({ + propertyChange: expect.objectContaining({ + nestedPropertyChanges: expect.arrayContaining([ + [ + 'exampleFunction', + expect.objectContaining({ + status: 'functionChange', + functionChangeLog: expect.objectContaining({ + parameterTypes: expect.objectContaining({ + nestedChanges: expect.arrayContaining([ + [ + 0, + 0, + expect.objectContaining({ + status: 'members', + memberLog: expect.objectContaining({ + memberKind: 'union', + addedMembers: expect.arrayContaining([ + expect.objectContaining({ + type: 'TypeAliasTypeAnnotation', + name: 'ObjectC', + }), + ]), + }), + }), + ], + ]), + }), + }), + }), + ], + ]), + }), + }), + ]), + }), + }), + ]), + }), + }), + ); + }); + + it('RN with union object types, removal is ok', () => { + const schemaDiff = buildSchemaDiff( + schemaWithUnionObject, // new: ObjectA | ObjectB + schemaWithUnionObjectAdded, // old: ObjectA | ObjectB | ObjectC + ); + + expect(summarizeDiffSet(schemaDiff).status).toBe('ok'); + }); + describe('NativeComponent', () => { it('module to component; deprecated + new', () => { expect( diff --git a/packages/react-native-compatibility-check/src/__tests__/__fixtures__/native-module-with-union-object-added/NativeModule.js.flow b/packages/react-native-compatibility-check/src/__tests__/__fixtures__/native-module-with-union-object-added/NativeModule.js.flow new file mode 100644 index 000000000000..e17b41dd1f24 --- /dev/null +++ b/packages/react-native-compatibility-check/src/__tests__/__fixtures__/native-module-with-union-object-added/NativeModule.js.flow @@ -0,0 +1,41 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + * @format + */ + +import type {TurboModule} from 'react-native/Libraries/TurboModule/RCTExport'; + +import * as TurboModuleRegistry from 'react-native/Libraries/TurboModule/TurboModuleRegistry'; + +type ObjectA = {| + kind: 'a', + value: number, +|}; + +type ObjectB = {| + kind: 'b', + name: string, +|}; + +type ObjectC = {| + kind: 'c', + flag: boolean, +|}; + +type ObjectUnion = ObjectA | ObjectB | ObjectC; + +export interface Spec extends TurboModule { + +exampleFunction: (a: ObjectUnion, b: number) => void; + +getConstants: () => { + +exampleConstant: number, + }; +} + +export default (TurboModuleRegistry.getEnforcing( + 'NativeModuleTest', +): Spec); diff --git a/packages/react-native-compatibility-check/src/__tests__/__snapshots__/ErrorFormatting-test.js.snap b/packages/react-native-compatibility-check/src/__tests__/__snapshots__/ErrorFormatting-test.js.snap index 756dea2572a4..98281373e4b0 100644 --- a/packages/react-native-compatibility-check/src/__tests__/__snapshots__/ErrorFormatting-test.js.snap +++ b/packages/react-native-compatibility-check/src/__tests__/__snapshots__/ErrorFormatting-test.js.snap @@ -42,6 +42,13 @@ Object { } `; +exports[`codegen formattedSummarizeDiffSet allows a diff from native-module-with-union-object-added/NativeModule to native-module-with-union-object/NativeModule 1`] = ` +Object { + "incompatibilityReport": Object {}, + "status": "ok", +} +`; + exports[`codegen formattedSummarizeDiffSet reports a diff from native-component-with-command/NativeComponent to native-component-with-command-extra-arg/NativeComponent 1`] = ` Object { "incompatibilityReport": Object { @@ -891,6 +898,31 @@ Object { } `; +exports[`codegen formattedSummarizeDiffSet reports a diff from native-module-with-union-object/NativeModule to native-module-with-union-object-added/NativeModule 1`] = ` +Object { + "incompatibilityReport": Object { + "NativeModule": Object { + "framework": "ReactNative", + "incompatibleSpecs": Array [ + Object { + "errorCode": "addedMemberCases", + "message": "NativeModuleTest: Object contained a property with a type mismatch + -- exampleFunction: has conflicting type changes + --new: (a: Union, b: number)=>void + --old: (a: Union, b: number)=>void + Parameter at index 0 did not match + --new: Union + --old: Union + Union added items, but native will not expect/support them + -- Member TypeAliasTypeAnnotation", + }, + ], + }, + }, + "status": "incompatible", +} +`; + exports[`codegen formattedSummarizeDiffSet reports a diff from native-module-with-union-object-changes/NativeModule to native-module-with-union-object/NativeModule 1`] = ` Object { "incompatibilityReport": Object {