Skip to content

Commit

Permalink
Merge pull request #381 from Workiva/fix-mocked-event-types
Browse files Browse the repository at this point in the history
FED-1881 Fix regression in SyntheticEvent mock class type-checking
  • Loading branch information
rmconsole2-wf authored Nov 9, 2023
2 parents 0218473 + 11e485e commit b27803c
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 5 deletions.
24 changes: 19 additions & 5 deletions lib/src/react_client/event_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -760,11 +760,25 @@ SyntheticWheelEvent createSyntheticWheelEvent({
}

extension SyntheticEventTypeHelpers on SyntheticEvent {
// Use getProperty(this, 'type') since, although statically we may be dealing with a SyntheticEvent,
// this could be a non-event JS object cast to SyntheticEvent with a null `type`.
// This is unlikely, but is possible, and before the null safety migration this method
// gracefully returned false instead of throwing.
bool _checkEventType(List<String> types) => getProperty(this, 'type') != null && types.any((t) => type.contains(t));
// Access `type` in a try-catch since, although statically we may be dealing with a SyntheticEvent,
// this could be an object with a `null` `type` which would cause a type error since `type` is non-nullable.
//
// Cases where this could occur:
// - non-event JS object cast to SyntheticEvent
// - a mock class that hasn't mocked `type`
//
// We could use `getProperty(this, 'type')` to handle the JS object case, but for mock classes it would bypass the
// `type` getter and not behave as expected.
bool _checkEventType(List<String> types) {
String? type;
try {
// This is typed as null statically, but if it's not, it will probably throw (depending on the compiler).
type = this.type;
} catch (_) {}

return type != null && types.any(type.contains);
}

bool _hasProperty(String propertyName) => hasProperty(this, propertyName);

/// Uses Duck Typing to detect if the event instance is a [SyntheticClipboardEvent].
Expand Down
2 changes: 2 additions & 0 deletions test/mockito.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ library react.test.mockito_gen_entrypoint;

import 'dart:html';
import 'package:mockito/annotations.dart';
import 'package:react/src/react_client/synthetic_event_wrappers.dart';

@GenerateNiceMocks([
MockSpec<KeyboardEvent>(),
MockSpec<DataTransfer>(),
MockSpec<MouseEvent>(),
MockSpec<SyntheticEvent>(),
])
main() {}
57 changes: 57 additions & 0 deletions test/mockito.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:html' as _i2;
import 'dart:math' as _i3;

import 'package:mockito/mockito.dart' as _i1;
import 'package:react/src/react_client/synthetic_event_wrappers.dart' as _i4;

// ignore_for_file: type=lint
// ignore_for_file: avoid_redundant_argument_values
Expand Down Expand Up @@ -444,3 +445,59 @@ class MockMouseEvent extends _i1.Mock implements _i2.MouseEvent {
returnValueForMissingStub: null,
);
}

/// A class which mocks [SyntheticEvent].
///
/// See the documentation for Mockito's code generation for more information.
class MockSyntheticEvent extends _i1.Mock implements _i4.SyntheticEvent {
@override
bool get bubbles => (super.noSuchMethod(
Invocation.getter(#bubbles),
returnValue: false,
returnValueForMissingStub: false,
) as bool);
@override
bool get cancelable => (super.noSuchMethod(
Invocation.getter(#cancelable),
returnValue: false,
returnValueForMissingStub: false,
) as bool);
@override
bool get defaultPrevented => (super.noSuchMethod(
Invocation.getter(#defaultPrevented),
returnValue: false,
returnValueForMissingStub: false,
) as bool);
@override
num get eventPhase => (super.noSuchMethod(
Invocation.getter(#eventPhase),
returnValue: 0,
returnValueForMissingStub: 0,
) as num);
@override
bool get isTrusted => (super.noSuchMethod(
Invocation.getter(#isTrusted),
returnValue: false,
returnValueForMissingStub: false,
) as bool);
@override
num get timeStamp => (super.noSuchMethod(
Invocation.getter(#timeStamp),
returnValue: 0,
returnValueForMissingStub: 0,
) as num);
@override
String get type => (super.noSuchMethod(
Invocation.getter(#type),
returnValue: '',
returnValueForMissingStub: '',
) as String);
@override
void preventDefault() => super.noSuchMethod(
Invocation.method(
#preventDefault,
[],
),
returnValueForMissingStub: null,
);
}
29 changes: 29 additions & 0 deletions test/react_client/event_helpers_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1703,6 +1703,18 @@ main() {
() {
expect(eventTypeTester(newObject() as SyntheticEvent), isFalse);
});

test('when the argument is a mocked event object with no mocked `type` property, and does not throw', () {
// Typically consumers would mock a specific SyntheticEvent subtype, but creating null-safe mocks for those
// causes property checks like `_hasProperty('button')` in helper methods to return true in DDC
// (e.g., `.isMouseEvent` for a `MockSyntheticMouseEvent` would return true).
//
// We really just want to check the `type` behavior here, especially for non-null-safe mocks, so we'll use
// the generic MockSyntheticEvent.
//
// *See other test with similar note to this one.*
expect(eventTypeTester(MockSyntheticEvent()), isFalse);
});
});
}

Expand Down Expand Up @@ -1990,6 +2002,23 @@ main() {
});
});
});

// Regression test for Mock class behavior consumers rely on.
//
// Typically consumers would mock a specific SyntheticEvent subtype, but creating null-safe mocks for those
// causes property checks like `_hasProperty('button')` in helper methods to return true in DDC
// (e.g., `.isMouseEvent` for a `MockSyntheticMouseEvent` would return true).
//
// We really just want to check the `type` behavior here, especially for non-null-safe mocks, so we'll use
// the generic MockSyntheticEvent.
//
// *See other test with similar note to this one.*
test('checks types correctly for Mock objects with `type` mocked', () {
final mockEvent = MockSyntheticEvent();
when(mockEvent.type).thenReturn('click');
expect(mockEvent.isMouseEvent, isTrue);
expect(mockEvent.isKeyboardEvent, false);
});
});

group('DataTransferHelper', () {
Expand Down

0 comments on commit b27803c

Please sign in to comment.