From ab59e1819af01f51de631a75cb03454455831de2 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 23 Jun 2020 13:43:04 -0700 Subject: [PATCH 01/49] EventEmitter: Upgrade from `require` to `import` Summary: Upgrades dependents of `EventEmitter`, `EventSubscription`, `EventSubscriptionVendor`, and `EmitterSubscription` to use `import` instead of `require`. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: cpojer Differential Revision: D22182312 fbshipit-source-id: e9444aa2728d89d52f577725f688871f7dbfba8a --- Libraries/AppState/AppState.js | 2 +- Libraries/EventEmitter/NativeEventEmitter.js | 2 +- Libraries/EventEmitter/RCTDeviceEventEmitter.js | 5 ++--- Libraries/EventEmitter/__mocks__/NativeEventEmitter.js | 3 ++- Libraries/Interaction/InteractionManager.js | 3 ++- Libraries/ReactNative/AppContainer.js | 5 +++-- Libraries/vendor/emitter/EventEmitter.js | 6 +++--- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Libraries/AppState/AppState.js b/Libraries/AppState/AppState.js index b5fb59bd9f..bdd799bf4e 100644 --- a/Libraries/AppState/AppState.js +++ b/Libraries/AppState/AppState.js @@ -10,12 +10,12 @@ 'use strict'; -const EventEmitter = require('../vendor/emitter/EventEmitter'); const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); const invariant = require('invariant'); const logError = require('../Utilities/logError'); +import EventEmitter from '../vendor/emitter/EventEmitter'; import NativeAppState from './NativeAppState'; /** diff --git a/Libraries/EventEmitter/NativeEventEmitter.js b/Libraries/EventEmitter/NativeEventEmitter.js index 59e03aa47d..4f47d17d40 100644 --- a/Libraries/EventEmitter/NativeEventEmitter.js +++ b/Libraries/EventEmitter/NativeEventEmitter.js @@ -10,13 +10,13 @@ 'use strict'; -const EventEmitter = require('../vendor/emitter/EventEmitter'); const Platform = require('../Utilities/Platform'); const RCTDeviceEventEmitter = require('./RCTDeviceEventEmitter'); const invariant = require('invariant'); import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; +import EventEmitter from '../vendor/emitter/EventEmitter'; type NativeModule = { +addListener: (eventType: string) => void, diff --git a/Libraries/EventEmitter/RCTDeviceEventEmitter.js b/Libraries/EventEmitter/RCTDeviceEventEmitter.js index 0b4c15d501..7e71afcf05 100644 --- a/Libraries/EventEmitter/RCTDeviceEventEmitter.js +++ b/Libraries/EventEmitter/RCTDeviceEventEmitter.js @@ -10,10 +10,9 @@ 'use strict'; -const EventEmitter = require('../vendor/emitter/EventEmitter'); -const EventSubscriptionVendor = require('../vendor/emitter/EventSubscriptionVendor'); - import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; +import EventEmitter from '../vendor/emitter/EventEmitter'; +import EventSubscriptionVendor from '../vendor/emitter/EventSubscriptionVendor'; function checkNativeEventModule(eventType: ?string) { if (eventType) { diff --git a/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js b/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js index 5a8eadc279..c507d72afb 100644 --- a/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js +++ b/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js @@ -10,9 +10,10 @@ 'use strict'; -const EventEmitter = require('../../vendor/emitter/EventEmitter'); const RCTDeviceEventEmitter = require('../RCTDeviceEventEmitter'); +import EventEmitter from '../../vendor/emitter/EventEmitter'; + /** * Mock the NativeEventEmitter as a normal JS EventEmitter. */ diff --git a/Libraries/Interaction/InteractionManager.js b/Libraries/Interaction/InteractionManager.js index 02912bbc3c..1bf5cbf032 100644 --- a/Libraries/Interaction/InteractionManager.js +++ b/Libraries/Interaction/InteractionManager.js @@ -11,13 +11,14 @@ 'use strict'; const BatchedBridge = require('../BatchedBridge/BatchedBridge'); -const EventEmitter = require('../vendor/emitter/EventEmitter'); const TaskQueue = require('./TaskQueue'); const infoLog = require('../Utilities/infoLog'); const invariant = require('invariant'); const keyMirror = require('fbjs/lib/keyMirror'); +import EventEmitter from '../vendor/emitter/EventEmitter'; + export type Handle = number; import type {Task} from './TaskQueue'; diff --git a/Libraries/ReactNative/AppContainer.js b/Libraries/ReactNative/AppContainer.js index 2c2b0fc157..544a6d3571 100644 --- a/Libraries/ReactNative/AppContainer.js +++ b/Libraries/ReactNative/AppContainer.js @@ -10,14 +10,15 @@ 'use strict'; -const EmitterSubscription = require('../vendor/emitter/EmitterSubscription'); const PropTypes = require('prop-types'); const RCTDeviceEventEmitter = require('../EventEmitter/RCTDeviceEventEmitter'); const React = require('react'); -import {RootTagContext, createRootTag} from './RootTag'; const StyleSheet = require('../StyleSheet/StyleSheet'); const View = require('../Components/View/View'); +import EmitterSubscription from '../vendor/emitter/EmitterSubscription'; +import {RootTagContext, createRootTag} from './RootTag'; + type Context = {rootTag: number, ...}; type Props = $ReadOnly<{| diff --git a/Libraries/vendor/emitter/EventEmitter.js b/Libraries/vendor/emitter/EventEmitter.js index f3c18c9105..d2ac83a969 100644 --- a/Libraries/vendor/emitter/EventEmitter.js +++ b/Libraries/vendor/emitter/EventEmitter.js @@ -11,11 +11,11 @@ 'use strict'; -const EmitterSubscription = require('./EmitterSubscription'); -const EventSubscriptionVendor = require('./EventSubscriptionVendor'); - const invariant = require('invariant'); +import EmitterSubscription from './EmitterSubscription'; +import EventSubscriptionVendor from './EventSubscriptionVendor'; + const sparseFilterPredicate = () => true; /** From 15911acd83e05d6d81fe966c9651ecb84e1e5f66 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 23 Jun 2020 13:43:04 -0700 Subject: [PATCH 02/49] EventEmitter: Export `EventSubscription` Interface Summary: Changes `EventEmitter.js` to be Flow-typed so that it can export an interface for `EventSubscription`. In order to retain the untyped-ness of `EventEmitter.js`, I moved the entire definition into an untyped `_EventEmitter.js` file (which I hope no one will try importing from). This new interface will be used to replace all the current type imports of `EventSubscription` and `EmitterSubscription`. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: cpojer Differential Revision: D22182311 fbshipit-source-id: c604a371a91963116621e89ca63f5a82167090b9 --- Libraries/vendor/emitter/EventEmitter.js | 220 +-------------------- Libraries/vendor/emitter/_EventEmitter.js | 228 ++++++++++++++++++++++ 2 files changed, 234 insertions(+), 214 deletions(-) create mode 100644 Libraries/vendor/emitter/_EventEmitter.js diff --git a/Libraries/vendor/emitter/EventEmitter.js b/Libraries/vendor/emitter/EventEmitter.js index d2ac83a969..3057ea4cb5 100644 --- a/Libraries/vendor/emitter/EventEmitter.js +++ b/Libraries/vendor/emitter/EventEmitter.js @@ -4,225 +4,17 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * + * @flow strict-local * @format - * @noflow - * @typecheck */ 'use strict'; -const invariant = require('invariant'); +// $FlowFixMe - EventEmitter Type Safety +const EventEmitter = require('./_EventEmitter'); -import EmitterSubscription from './EmitterSubscription'; -import EventSubscriptionVendor from './EventSubscriptionVendor'; +export default EventEmitter; -const sparseFilterPredicate = () => true; - -/** - * @class EventEmitter - * @description - * An EventEmitter is responsible for managing a set of listeners and publishing - * events to them when it is told that such events happened. In addition to the - * data for the given event it also sends a event control object which allows - * the listeners/handlers to prevent the default behavior of the given event. - * - * The emitter is designed to be generic enough to support all the different - * contexts in which one might want to emit events. It is a simple multicast - * mechanism on top of which extra functionality can be composed. For example, a - * more advanced emitter may use an EventHolder and EventFactory. - */ -class EventEmitter { - _subscriber: EventSubscriptionVendor; - _currentSubscription: ?EmitterSubscription; - - /** - * @constructor - * - * @param {EventSubscriptionVendor} subscriber - Optional subscriber instance - * to use. If omitted, a new subscriber will be created for the emitter. - */ - constructor(subscriber: ?EventSubscriptionVendor) { - this._subscriber = subscriber || new EventSubscriptionVendor(); - } - - /** - * Adds a listener to be invoked when events of the specified type are - * emitted. An optional calling context may be provided. The data arguments - * emitted will be passed to the listener function. - * - * TODO: Annotate the listener arg's type. This is tricky because listeners - * can be invoked with varargs. - * - * @param {string} eventType - Name of the event to listen to - * @param {function} listener - Function to invoke when the specified event is - * emitted - * @param {*} context - Optional context object to use when invoking the - * listener - */ - addListener( - eventType: string, - listener: Function, - context: ?Object, - ): EmitterSubscription { - return (this._subscriber.addSubscription( - eventType, - new EmitterSubscription(this, this._subscriber, listener, context), - ): any); - } - - /** - * Similar to addListener, except that the listener is removed after it is - * invoked once. - * - * @param {string} eventType - Name of the event to listen to - * @param {function} listener - Function to invoke only once when the - * specified event is emitted - * @param {*} context - Optional context object to use when invoking the - * listener - */ - once( - eventType: string, - listener: Function, - context: ?Object, - ): EmitterSubscription { - return this.addListener(eventType, (...args) => { - this.removeCurrentListener(); - listener.apply(context, args); - }); - } - - /** - * Removes all of the registered listeners, including those registered as - * listener maps. - * - * @param {?string} eventType - Optional name of the event whose registered - * listeners to remove - */ - removeAllListeners(eventType: ?string) { - this._subscriber.removeAllSubscriptions(eventType); - } - - /** - * Provides an API that can be called during an eventing cycle to remove the - * last listener that was invoked. This allows a developer to provide an event - * object that can remove the listener (or listener map) during the - * invocation. - * - * If it is called when not inside of an emitting cycle it will throw. - * - * @throws {Error} When called not during an eventing cycle - * - * @example - * var subscription = emitter.addListenerMap({ - * someEvent: function(data, event) { - * console.log(data); - * emitter.removeCurrentListener(); - * } - * }); - * - * emitter.emit('someEvent', 'abc'); // logs 'abc' - * emitter.emit('someEvent', 'def'); // does not log anything - */ - removeCurrentListener() { - invariant( - !!this._currentSubscription, - 'Not in an emitting cycle; there is no current subscription', - ); - this.removeSubscription(this._currentSubscription); - } - - /** - * Removes a specific subscription. Called by the `remove()` method of the - * subscription itself to ensure any necessary cleanup is performed. - */ - removeSubscription(subscription: EmitterSubscription) { - invariant( - subscription.emitter === this, - 'Subscription does not belong to this emitter.', - ); - this._subscriber.removeSubscription(subscription); - } - - /** - * Returns an array of listeners that are currently registered for the given - * event. - * - * @param {string} eventType - Name of the event to query - * @returns {array} - */ - listeners(eventType: string): [EmitterSubscription] { - const subscriptions = this._subscriber.getSubscriptionsForType(eventType); - return subscriptions - ? subscriptions - // We filter out missing entries because the array is sparse. - // "callbackfn is called only for elements of the array which actually - // exist; it is not called for missing elements of the array." - // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.filter - .filter(sparseFilterPredicate) - .map(subscription => subscription.listener) - : []; - } - - /** - * Emits an event of the given type with the given data. All handlers of that - * particular type will be notified. - * - * @param {string} eventType - Name of the event to emit - * @param {...*} Arbitrary arguments to be passed to each registered listener - * - * @example - * emitter.addListener('someEvent', function(message) { - * console.log(message); - * }); - * - * emitter.emit('someEvent', 'abc'); // logs 'abc' - */ - emit(eventType: string) { - const subscriptions = this._subscriber.getSubscriptionsForType(eventType); - if (subscriptions) { - for (let i = 0, l = subscriptions.length; i < l; i++) { - const subscription = subscriptions[i]; - - // The subscription may have been removed during this event loop. - if (subscription && subscription.listener) { - this._currentSubscription = subscription; - subscription.listener.apply( - subscription.context, - Array.prototype.slice.call(arguments, 1), - ); - } - } - this._currentSubscription = null; - } - } - - /** - * Removes the given listener for event of specific type. - * - * @param {string} eventType - Name of the event to emit - * @param {function} listener - Function to invoke when the specified event is - * emitted - * - * @example - * emitter.removeListener('someEvent', function(message) { - * console.log(message); - * }); // removes the listener if already registered - * - */ - removeListener(eventType: String, listener) { - const subscriptions = this._subscriber.getSubscriptionsForType(eventType); - if (subscriptions) { - for (let i = 0, l = subscriptions.length; i < l; i++) { - const subscription = subscriptions[i]; - - // The subscription may have been removed during this event loop. - // its listener matches the listener in method parameters - if (subscription && subscription.listener === listener) { - subscription.remove(); - } - } - } - } +export interface EventSubscription { + remove(): void; } - -module.exports = EventEmitter; diff --git a/Libraries/vendor/emitter/_EventEmitter.js b/Libraries/vendor/emitter/_EventEmitter.js new file mode 100644 index 0000000000..d2ac83a969 --- /dev/null +++ b/Libraries/vendor/emitter/_EventEmitter.js @@ -0,0 +1,228 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @noflow + * @typecheck + */ + +'use strict'; + +const invariant = require('invariant'); + +import EmitterSubscription from './EmitterSubscription'; +import EventSubscriptionVendor from './EventSubscriptionVendor'; + +const sparseFilterPredicate = () => true; + +/** + * @class EventEmitter + * @description + * An EventEmitter is responsible for managing a set of listeners and publishing + * events to them when it is told that such events happened. In addition to the + * data for the given event it also sends a event control object which allows + * the listeners/handlers to prevent the default behavior of the given event. + * + * The emitter is designed to be generic enough to support all the different + * contexts in which one might want to emit events. It is a simple multicast + * mechanism on top of which extra functionality can be composed. For example, a + * more advanced emitter may use an EventHolder and EventFactory. + */ +class EventEmitter { + _subscriber: EventSubscriptionVendor; + _currentSubscription: ?EmitterSubscription; + + /** + * @constructor + * + * @param {EventSubscriptionVendor} subscriber - Optional subscriber instance + * to use. If omitted, a new subscriber will be created for the emitter. + */ + constructor(subscriber: ?EventSubscriptionVendor) { + this._subscriber = subscriber || new EventSubscriptionVendor(); + } + + /** + * Adds a listener to be invoked when events of the specified type are + * emitted. An optional calling context may be provided. The data arguments + * emitted will be passed to the listener function. + * + * TODO: Annotate the listener arg's type. This is tricky because listeners + * can be invoked with varargs. + * + * @param {string} eventType - Name of the event to listen to + * @param {function} listener - Function to invoke when the specified event is + * emitted + * @param {*} context - Optional context object to use when invoking the + * listener + */ + addListener( + eventType: string, + listener: Function, + context: ?Object, + ): EmitterSubscription { + return (this._subscriber.addSubscription( + eventType, + new EmitterSubscription(this, this._subscriber, listener, context), + ): any); + } + + /** + * Similar to addListener, except that the listener is removed after it is + * invoked once. + * + * @param {string} eventType - Name of the event to listen to + * @param {function} listener - Function to invoke only once when the + * specified event is emitted + * @param {*} context - Optional context object to use when invoking the + * listener + */ + once( + eventType: string, + listener: Function, + context: ?Object, + ): EmitterSubscription { + return this.addListener(eventType, (...args) => { + this.removeCurrentListener(); + listener.apply(context, args); + }); + } + + /** + * Removes all of the registered listeners, including those registered as + * listener maps. + * + * @param {?string} eventType - Optional name of the event whose registered + * listeners to remove + */ + removeAllListeners(eventType: ?string) { + this._subscriber.removeAllSubscriptions(eventType); + } + + /** + * Provides an API that can be called during an eventing cycle to remove the + * last listener that was invoked. This allows a developer to provide an event + * object that can remove the listener (or listener map) during the + * invocation. + * + * If it is called when not inside of an emitting cycle it will throw. + * + * @throws {Error} When called not during an eventing cycle + * + * @example + * var subscription = emitter.addListenerMap({ + * someEvent: function(data, event) { + * console.log(data); + * emitter.removeCurrentListener(); + * } + * }); + * + * emitter.emit('someEvent', 'abc'); // logs 'abc' + * emitter.emit('someEvent', 'def'); // does not log anything + */ + removeCurrentListener() { + invariant( + !!this._currentSubscription, + 'Not in an emitting cycle; there is no current subscription', + ); + this.removeSubscription(this._currentSubscription); + } + + /** + * Removes a specific subscription. Called by the `remove()` method of the + * subscription itself to ensure any necessary cleanup is performed. + */ + removeSubscription(subscription: EmitterSubscription) { + invariant( + subscription.emitter === this, + 'Subscription does not belong to this emitter.', + ); + this._subscriber.removeSubscription(subscription); + } + + /** + * Returns an array of listeners that are currently registered for the given + * event. + * + * @param {string} eventType - Name of the event to query + * @returns {array} + */ + listeners(eventType: string): [EmitterSubscription] { + const subscriptions = this._subscriber.getSubscriptionsForType(eventType); + return subscriptions + ? subscriptions + // We filter out missing entries because the array is sparse. + // "callbackfn is called only for elements of the array which actually + // exist; it is not called for missing elements of the array." + // https://www.ecma-international.org/ecma-262/9.0/index.html#sec-array.prototype.filter + .filter(sparseFilterPredicate) + .map(subscription => subscription.listener) + : []; + } + + /** + * Emits an event of the given type with the given data. All handlers of that + * particular type will be notified. + * + * @param {string} eventType - Name of the event to emit + * @param {...*} Arbitrary arguments to be passed to each registered listener + * + * @example + * emitter.addListener('someEvent', function(message) { + * console.log(message); + * }); + * + * emitter.emit('someEvent', 'abc'); // logs 'abc' + */ + emit(eventType: string) { + const subscriptions = this._subscriber.getSubscriptionsForType(eventType); + if (subscriptions) { + for (let i = 0, l = subscriptions.length; i < l; i++) { + const subscription = subscriptions[i]; + + // The subscription may have been removed during this event loop. + if (subscription && subscription.listener) { + this._currentSubscription = subscription; + subscription.listener.apply( + subscription.context, + Array.prototype.slice.call(arguments, 1), + ); + } + } + this._currentSubscription = null; + } + } + + /** + * Removes the given listener for event of specific type. + * + * @param {string} eventType - Name of the event to emit + * @param {function} listener - Function to invoke when the specified event is + * emitted + * + * @example + * emitter.removeListener('someEvent', function(message) { + * console.log(message); + * }); // removes the listener if already registered + * + */ + removeListener(eventType: String, listener) { + const subscriptions = this._subscriber.getSubscriptionsForType(eventType); + if (subscriptions) { + for (let i = 0, l = subscriptions.length; i < l; i++) { + const subscription = subscriptions[i]; + + // The subscription may have been removed during this event loop. + // its listener matches the listener in method parameters + if (subscription && subscription.listener === listener) { + subscription.remove(); + } + } + } + } +} + +module.exports = EventEmitter; From 331d3268e2bac2941bfd1e8cd20331ac870702bd Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 23 Jun 2020 13:43:04 -0700 Subject: [PATCH 03/49] EventEmitter: Import `EventSubscription` from `EventEmitter` Summary: Changes dependents to import the `EventSubscription` interface type from `EventEmitter` instead of `EventSubscription.js`. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: cpojer Differential Revision: D22182313 fbshipit-source-id: 31448ea5ee3038f806a417f23597da1edd4e4e9b --- Libraries/WebSocket/WebSocket.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Libraries/WebSocket/WebSocket.js b/Libraries/WebSocket/WebSocket.js index 0b051c48e4..5023a0b07f 100644 --- a/Libraries/WebSocket/WebSocket.js +++ b/Libraries/WebSocket/WebSocket.js @@ -20,7 +20,7 @@ const base64 = require('base64-js'); const binaryToBase64 = require('../Utilities/binaryToBase64'); const invariant = require('invariant'); -import type EventSubscription from '../vendor/emitter/EventSubscription'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import NativeWebSocketModule from './NativeWebSocketModule'; type ArrayBufferView = From 70a73ca461643d1680a24482d8f69387ed43a71c Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 23 Jun 2020 13:43:04 -0700 Subject: [PATCH 04/49] EventEmitter: Import `{Emitter => Event}Subscription` Summary: Changes dependents of `EmitterSubscription` to instead import the `EventSubscription` type from `EventEmitter.js`. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: cpojer Differential Revision: D22182309 fbshipit-source-id: 575f4c59248ef50182ddb33911f1e6f3ba88ec07 --- .../ReactContentSizeUpdateTest.js | 4 ++-- IntegrationTests/SizeFlexibilityUpdateTest.js | 4 ++-- Libraries/BugReporting/BugReporting.js | 8 ++++---- .../Components/AppleTV/TVEventHandler.js | 6 +++--- .../Keyboard/KeyboardAvoidingView.js | 4 ++-- Libraries/Components/ScrollResponder.js | 20 +++++++++---------- Libraries/EventEmitter/NativeEventEmitter.js | 6 +++--- .../EventEmitter/RCTDeviceEventEmitter.js | 2 +- Libraries/Modal/Modal.js | 13 ++++++------ Libraries/ReactNative/AppContainer.js | 4 ++-- .../OrientationChangeExample.js | 4 ++-- 11 files changed, 38 insertions(+), 37 deletions(-) diff --git a/IntegrationTests/ReactContentSizeUpdateTest.js b/IntegrationTests/ReactContentSizeUpdateTest.js index eefb6681b2..7abcc4fb8b 100644 --- a/IntegrationTests/ReactContentSizeUpdateTest.js +++ b/IntegrationTests/ReactContentSizeUpdateTest.js @@ -17,7 +17,7 @@ const ReactNative = require('react-native'); const {View} = ReactNative; const {TestModule} = ReactNative.NativeModules; -import type EmitterSubscription from 'react-native/Libraries/vendor/emitter/EmitterSubscription'; +import {type EventSubscription} from 'react-native/Libraries/vendor/emitter/EventEmitter'; const reactViewWidth = 101; const reactViewHeight = 102; @@ -33,7 +33,7 @@ type State = {| class ReactContentSizeUpdateTest extends React.Component { _timeoutID: ?TimeoutID = null; - _subscription: ?EmitterSubscription = null; + _subscription: ?EventSubscription = null; state: State = { height: reactViewHeight, diff --git a/IntegrationTests/SizeFlexibilityUpdateTest.js b/IntegrationTests/SizeFlexibilityUpdateTest.js index fe7a294485..b2e4952b10 100644 --- a/IntegrationTests/SizeFlexibilityUpdateTest.js +++ b/IntegrationTests/SizeFlexibilityUpdateTest.js @@ -16,7 +16,7 @@ const ReactNative = require('react-native'); const {View} = ReactNative; const {TestModule} = ReactNative.NativeModules; -import type EmitterSubscription from 'react-native/Libraries/vendor/emitter/EmitterSubscription'; +import {type EventSubscription} from 'react-native/Libraries/vendor/emitter/EventEmitter'; const reactViewWidth = 111; const reactViewHeight = 222; @@ -31,7 +31,7 @@ type Props = $ReadOnly<{| |}>; class SizeFlexibilityUpdateTest extends React.Component { - _subscription: ?EmitterSubscription = null; + _subscription: ?EventSubscription = null; UNSAFE_componentWillMount() { this._subscription = RCTNativeAppEventEmitter.addListener( diff --git a/Libraries/BugReporting/BugReporting.js b/Libraries/BugReporting/BugReporting.js index 001c44c17b..e6ccae6cc3 100644 --- a/Libraries/BugReporting/BugReporting.js +++ b/Libraries/BugReporting/BugReporting.js @@ -12,9 +12,9 @@ const RCTDeviceEventEmitter = require('../EventEmitter/RCTDeviceEventEmitter'); -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; -import NativeBugReporting from './NativeBugReporting'; import NativeRedBox from '../NativeModules/specs/NativeRedBox'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; +import NativeBugReporting from './NativeBugReporting'; type ExtraData = {[key: string]: string, ...}; type SourceCallback = () => string; @@ -39,8 +39,8 @@ function defaultExtras() { class BugReporting { static _extraSources: Map = new Map(); static _fileSources: Map = new Map(); - static _subscription: ?EmitterSubscription = null; - static _redboxSubscription: ?EmitterSubscription = null; + static _subscription: ?EventSubscription = null; + static _redboxSubscription: ?EventSubscription = null; static _maybeInit() { if (!BugReporting._subscription) { diff --git a/Libraries/Components/AppleTV/TVEventHandler.js b/Libraries/Components/AppleTV/TVEventHandler.js index d5cd47afdc..a7f12b5306 100644 --- a/Libraries/Components/AppleTV/TVEventHandler.js +++ b/Libraries/Components/AppleTV/TVEventHandler.js @@ -10,14 +10,14 @@ 'use strict'; -const Platform = require('../../Utilities/Platform'); const NativeEventEmitter = require('../../EventEmitter/NativeEventEmitter'); +const Platform = require('../../Utilities/Platform'); +import {type EventSubscription} from '../../vendor/emitter/EventEmitter'; import NativeTVNavigationEventEmitter from './NativeTVNavigationEventEmitter'; -import type EmitterSubscription from '../../vendor/emitter/EmitterSubscription'; class TVEventHandler { - __nativeTVNavigationEventListener: ?EmitterSubscription = null; + __nativeTVNavigationEventListener: ?EventSubscription = null; __nativeTVNavigationEventEmitter: ?NativeEventEmitter = null; enable(component: ?any, callback: Function): void { diff --git a/Libraries/Components/Keyboard/KeyboardAvoidingView.js b/Libraries/Components/Keyboard/KeyboardAvoidingView.js index 07513e253b..b8a150efba 100644 --- a/Libraries/Components/Keyboard/KeyboardAvoidingView.js +++ b/Libraries/Components/Keyboard/KeyboardAvoidingView.js @@ -18,7 +18,7 @@ const StyleSheet = require('../../StyleSheet/StyleSheet'); const View = require('../View/View'); import type {ViewStyleProp} from '../../StyleSheet/StyleSheet'; -import type EmitterSubscription from '../../vendor/emitter/EmitterSubscription'; +import {type EventSubscription} from '../../vendor/emitter/EventEmitter'; import type { ViewProps, ViewLayout, @@ -67,7 +67,7 @@ class KeyboardAvoidingView extends React.Component { }; _frame: ?ViewLayout = null; - _subscriptions: Array = []; + _subscriptions: Array = []; viewRef: {current: React.ElementRef | null, ...}; _initialFrameHeight: number = 0; diff --git a/Libraries/Components/ScrollResponder.js b/Libraries/Components/ScrollResponder.js index a520d17d28..527db47eb7 100644 --- a/Libraries/Components/ScrollResponder.js +++ b/Libraries/Components/ScrollResponder.js @@ -10,25 +10,25 @@ 'use strict'; -const React = require('react'); const Dimensions = require('../Utilities/Dimensions'); const FrameRateLogger = require('../Interaction/FrameRateLogger'); const Keyboard = require('./Keyboard/Keyboard'); +const Platform = require('../Utilities/Platform'); +const React = require('react'); const ReactNative = require('../Renderer/shims/ReactNative'); const TextInputState = require('./TextInput/TextInputState'); const UIManager = require('../ReactNative/UIManager'); -const Platform = require('../Utilities/Platform'); -import Commands from './ScrollView/ScrollViewCommands'; const invariant = require('invariant'); const performanceNow = require('fbjs/lib/performanceNow'); +import type {HostComponent} from '../Renderer/shims/ReactNativeTypes'; import type {PressEvent, ScrollEvent} from '../Types/CoreEventTypes'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; +import type {KeyboardEvent} from './Keyboard/Keyboard'; import typeof ScrollView from './ScrollView/ScrollView'; import type {Props as ScrollViewProps} from './ScrollView/ScrollView'; -import type {KeyboardEvent} from './Keyboard/Keyboard'; -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; -import type {HostComponent} from '../Renderer/shims/ReactNativeTypes'; +import Commands from './ScrollView/ScrollViewCommands'; /** * Mixin that can be integrated in order to handle scrolling that plays well @@ -119,10 +119,10 @@ export type State = {| |}; const ScrollResponderMixin = { - _subscriptionKeyboardWillShow: (null: ?EmitterSubscription), - _subscriptionKeyboardWillHide: (null: ?EmitterSubscription), - _subscriptionKeyboardDidShow: (null: ?EmitterSubscription), - _subscriptionKeyboardDidHide: (null: ?EmitterSubscription), + _subscriptionKeyboardWillShow: (null: ?EventSubscription), + _subscriptionKeyboardWillHide: (null: ?EventSubscription), + _subscriptionKeyboardDidShow: (null: ?EventSubscription), + _subscriptionKeyboardDidHide: (null: ?EventSubscription), scrollResponderMixinGetInitialState: function(): State { return { isTouching: false, diff --git a/Libraries/EventEmitter/NativeEventEmitter.js b/Libraries/EventEmitter/NativeEventEmitter.js index 4f47d17d40..8ff7ab7724 100644 --- a/Libraries/EventEmitter/NativeEventEmitter.js +++ b/Libraries/EventEmitter/NativeEventEmitter.js @@ -15,8 +15,8 @@ const RCTDeviceEventEmitter = require('./RCTDeviceEventEmitter'); const invariant = require('invariant'); -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; import EventEmitter from '../vendor/emitter/EventEmitter'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; type NativeModule = { +addListener: (eventType: string) => void, @@ -43,7 +43,7 @@ class NativeEventEmitter extends EventEmitter { eventType: string, listener: Function, context: ?Object, - ): EmitterSubscription { + ): EventSubscription { if (this._nativeModule != null) { this._nativeModule.addListener(eventType); } @@ -59,7 +59,7 @@ class NativeEventEmitter extends EventEmitter { super.removeAllListeners(eventType); } - removeSubscription(subscription: EmitterSubscription) { + removeSubscription(subscription: EventSubscription) { if (this._nativeModule != null) { this._nativeModule.removeListeners(1); } diff --git a/Libraries/EventEmitter/RCTDeviceEventEmitter.js b/Libraries/EventEmitter/RCTDeviceEventEmitter.js index 7e71afcf05..41c16d3be8 100644 --- a/Libraries/EventEmitter/RCTDeviceEventEmitter.js +++ b/Libraries/EventEmitter/RCTDeviceEventEmitter.js @@ -10,8 +10,8 @@ 'use strict'; -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; import EventEmitter from '../vendor/emitter/EventEmitter'; +import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; import EventSubscriptionVendor from '../vendor/emitter/EventSubscriptionVendor'; function checkNativeEventModule(eventType: ?string) { diff --git a/Libraries/Modal/Modal.js b/Libraries/Modal/Modal.js index 3e9f0331a8..caefe6a3dc 100644 --- a/Libraries/Modal/Modal.js +++ b/Libraries/Modal/Modal.js @@ -11,19 +11,20 @@ 'use strict'; const AppContainer = require('../ReactNative/AppContainer'); -const {RootTagContext} = require('../ReactNative/RootTag'); const I18nManager = require('../ReactNative/I18nManager'); const React = require('react'); const ScrollView = require('../Components/ScrollView/ScrollView'); const StyleSheet = require('../StyleSheet/StyleSheet'); const View = require('../Components/View/View'); -import type {RootTag} from '../ReactNative/RootTag'; +const {RootTagContext} = require('../ReactNative/RootTag'); + import type {ViewProps} from '../Components/View/ViewPropTypes'; -import type {DirectEventHandler} from '../Types/CodegenTypes'; -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; -import RCTModalHostView from './RCTModalHostViewNativeComponent'; import {VirtualizedListContextResetter} from '../Lists/VirtualizedListContext.js'; +import type {RootTag} from '../ReactNative/RootTag'; +import type {DirectEventHandler} from '../Types/CodegenTypes'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; +import RCTModalHostView from './RCTModalHostViewNativeComponent'; /** * The Modal component is a simple way to present content above an enclosing view. @@ -152,7 +153,7 @@ class Modal extends React.Component { static contextType: React.Context = RootTagContext; _identifier: number; - _eventSubscription: ?EmitterSubscription; + _eventSubscription: ?EventSubscription; constructor(props: Props) { super(props); diff --git a/Libraries/ReactNative/AppContainer.js b/Libraries/ReactNative/AppContainer.js index 544a6d3571..66eafadc79 100644 --- a/Libraries/ReactNative/AppContainer.js +++ b/Libraries/ReactNative/AppContainer.js @@ -16,7 +16,7 @@ const React = require('react'); const StyleSheet = require('../StyleSheet/StyleSheet'); const View = require('../Components/View/View'); -import EmitterSubscription from '../vendor/emitter/EmitterSubscription'; +import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import {RootTagContext, createRootTag} from './RootTag'; type Context = {rootTag: number, ...}; @@ -44,7 +44,7 @@ class AppContainer extends React.Component { hasError: false, }; _mainRef: ?React.ElementRef; - _subscription: ?EmitterSubscription = null; + _subscription: ?EventSubscription = null; static getDerivedStateFromError: any = undefined; diff --git a/RNTester/js/examples/OrientationChange/OrientationChangeExample.js b/RNTester/js/examples/OrientationChange/OrientationChangeExample.js index 98e3de6348..d7ddd4f427 100644 --- a/RNTester/js/examples/OrientationChange/OrientationChangeExample.js +++ b/RNTester/js/examples/OrientationChange/OrientationChangeExample.js @@ -14,10 +14,10 @@ const React = require('react'); const {DeviceEventEmitter, Text, View} = require('react-native'); -import type EmitterSubscription from '../../../../Libraries/vendor/emitter/EmitterSubscription'; +import {type EventSubscription} from '../../../../Libraries/vendor/emitter/EventEmitter'; class OrientationChangeExample extends React.Component<{...}, $FlowFixMeState> { - _orientationSubscription: EmitterSubscription; + _orientationSubscription: EventSubscription; state = { currentOrientation: '', From 2933b887cd425061192489dcef81816644957f4d Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Tue, 23 Jun 2020 13:43:04 -0700 Subject: [PATCH 05/49] EventEmitter: Prefix Private Modules with `_` Summary: Renames `EventSubscription.js`, `EmitterSubscription.js`, and `EventSubscriptionVendor.js` to `_EventSubscription.js`, `_EmitterSubscription.js`, and `_EventSubscriptionVendor.js`, respectively. This is to indicate that those files are implementation details and should not be directly referenced. Instead, the `EventSubscription` type exported from `EventEmitter.js` should be used. The remaining stragglers that are importing `_EmitterSubscription.js` and `_EventSubscriptionVendor.js` after this commit will be cleaned up when `RCTDeviceEventEmitter` is refactored. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: cpojer Differential Revision: D22182310 fbshipit-source-id: 82be685f231395bd7b8e9986141b5df1367bec71 --- Libraries/EventEmitter/RCTDeviceEventEmitter.js | 4 ++-- .../{EmitterSubscription.js => _EmitterSubscription.js} | 5 ++--- Libraries/vendor/emitter/_EventEmitter.js | 4 ++-- .../emitter/{EventSubscription.js => _EventSubscription.js} | 2 +- ...ventSubscriptionVendor.js => _EventSubscriptionVendor.js} | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) rename Libraries/vendor/emitter/{EmitterSubscription.js => _EmitterSubscription.js} (92%) rename Libraries/vendor/emitter/{EventSubscription.js => _EventSubscription.js} (92%) rename Libraries/vendor/emitter/{EventSubscriptionVendor.js => _EventSubscriptionVendor.js} (97%) diff --git a/Libraries/EventEmitter/RCTDeviceEventEmitter.js b/Libraries/EventEmitter/RCTDeviceEventEmitter.js index 41c16d3be8..8c0f023287 100644 --- a/Libraries/EventEmitter/RCTDeviceEventEmitter.js +++ b/Libraries/EventEmitter/RCTDeviceEventEmitter.js @@ -11,8 +11,8 @@ 'use strict'; import EventEmitter from '../vendor/emitter/EventEmitter'; -import type EmitterSubscription from '../vendor/emitter/EmitterSubscription'; -import EventSubscriptionVendor from '../vendor/emitter/EventSubscriptionVendor'; +import type EmitterSubscription from '../vendor/emitter/_EmitterSubscription'; +import EventSubscriptionVendor from '../vendor/emitter/_EventSubscriptionVendor'; function checkNativeEventModule(eventType: ?string) { if (eventType) { diff --git a/Libraries/vendor/emitter/EmitterSubscription.js b/Libraries/vendor/emitter/_EmitterSubscription.js similarity index 92% rename from Libraries/vendor/emitter/EmitterSubscription.js rename to Libraries/vendor/emitter/_EmitterSubscription.js index cc3138305b..5a767c823d 100644 --- a/Libraries/vendor/emitter/EmitterSubscription.js +++ b/Libraries/vendor/emitter/_EmitterSubscription.js @@ -10,10 +10,9 @@ 'use strict'; -const EventSubscription = require('./EventSubscription'); - import type EventEmitter from './EventEmitter'; -import type EventSubscriptionVendor from './EventSubscriptionVendor'; +import EventSubscription from './_EventSubscription'; +import type EventSubscriptionVendor from './_EventSubscriptionVendor'; /** * EmitterSubscription represents a subscription with listener and context data. diff --git a/Libraries/vendor/emitter/_EventEmitter.js b/Libraries/vendor/emitter/_EventEmitter.js index d2ac83a969..b55f487ebe 100644 --- a/Libraries/vendor/emitter/_EventEmitter.js +++ b/Libraries/vendor/emitter/_EventEmitter.js @@ -13,8 +13,8 @@ const invariant = require('invariant'); -import EmitterSubscription from './EmitterSubscription'; -import EventSubscriptionVendor from './EventSubscriptionVendor'; +import EmitterSubscription from './_EmitterSubscription'; +import EventSubscriptionVendor from './_EventSubscriptionVendor'; const sparseFilterPredicate = () => true; diff --git a/Libraries/vendor/emitter/EventSubscription.js b/Libraries/vendor/emitter/_EventSubscription.js similarity index 92% rename from Libraries/vendor/emitter/EventSubscription.js rename to Libraries/vendor/emitter/_EventSubscription.js index 8f767e060c..54ec8f2f2d 100644 --- a/Libraries/vendor/emitter/EventSubscription.js +++ b/Libraries/vendor/emitter/_EventSubscription.js @@ -10,7 +10,7 @@ 'use strict'; -import type EventSubscriptionVendor from './EventSubscriptionVendor'; +import type EventSubscriptionVendor from './_EventSubscriptionVendor'; /** * EventSubscription represents a subscription to a particular event. It can diff --git a/Libraries/vendor/emitter/EventSubscriptionVendor.js b/Libraries/vendor/emitter/_EventSubscriptionVendor.js similarity index 97% rename from Libraries/vendor/emitter/EventSubscriptionVendor.js rename to Libraries/vendor/emitter/_EventSubscriptionVendor.js index db44c6f3ec..5ea0aa821e 100644 --- a/Libraries/vendor/emitter/EventSubscriptionVendor.js +++ b/Libraries/vendor/emitter/_EventSubscriptionVendor.js @@ -12,7 +12,7 @@ const invariant = require('invariant'); -import type EventSubscription from './EventSubscription'; +import type EventSubscription from './_EventSubscription'; /** * EventSubscriptionVendor stores a set of EventSubscriptions that are From 30054645c782e04fa0e34c1e48129f92c055e058 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 23 Jun 2020 15:00:20 -0700 Subject: [PATCH 06/49] Set clipped to YES when loading an image Summary: Changelog: [Internal] When Paper loads images. it calls with with `clipped` being set to NO. https://github.com/facebook/react-native/blob/master/Libraries/Image/RCTImageView.mm#L346 We should do the same in Fabric as it can result in different images being loaded. Reviewed By: JoshuaGross, shergin Differential Revision: D22185286 fbshipit-source-id: 04afafee67fdfac53e9d17f5edeaf9b9317c7a22 --- ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm b/ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm index 49a371bc41..9691162f65 100644 --- a/ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm +++ b/ReactCommon/fabric/imagemanager/platform/ios/RCTImageManager.mm @@ -88,7 +88,7 @@ using namespace facebook::react; [self->_imageLoader loadImageWithURLRequest:request size:CGSizeMake(imageSource.size.width, imageSource.size.height) scale:imageSource.scale - clipped:YES + clipped:NO resizeMode:RCTResizeModeStretch priority:RCTImageLoaderPriorityImmediate attribution:{ From 6f99beff7ea6e846b14b9080724fb91b19041567 Mon Sep 17 00:00:00 2001 From: Lulu Wu Date: Wed, 24 Jun 2020 06:00:22 -0700 Subject: [PATCH 07/49] Fix for Unsupported top level event type "onAssetDidLoad" Summary: Fix for Unsupported top level event type "onAssetDidLoad" Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D22184964 fbshipit-source-id: d1ce920311fb380923ed9a2ef8aa5a49fc9f8dc2 --- Libraries/Components/View/ReactNativeViewViewConfigAndroid.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js b/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js index d6ea59f56f..6f4056a0e7 100644 --- a/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js +++ b/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js @@ -57,6 +57,9 @@ const ReactNativeViewViewConfigAndroid = { topSelectionChange: { registrationName: 'onSelectionChange', }, + onAssetDidLoad: { + registrationName: 'onAssetDidLoad', + }, }, validAttributes: { hasTVPreferredFocus: true, From 208e4d2dcdbf5a2ef91294252ca3d5a039dff524 Mon Sep 17 00:00:00 2001 From: Vitaly Potlov Date: Wed, 24 Jun 2020 12:27:35 -0700 Subject: [PATCH 08/49] Fix static method in file header (#29118) Summary: I have looked into recent commits and found a typical mistake when static is used in .h file. Here static is not necessary. See link: https://stackoverflow.com/questions/780730/c-c-static-function-in-header-file-what-does-it-mean ## Changelog Changelog: [Internal] Fabric-specific internal change. Pull Request resolved: https://github.com/facebook/react-native/pull/29118 Reviewed By: sammy-SC Differential Revision: D22039305 Pulled By: shergin fbshipit-source-id: 7078e716166067dd1e94cbb84200a1235283c978 --- ReactCommon/utils/ManagedObjectWrapper.h | 13 +------------ ReactCommon/utils/ManagedObjectWrapper.mm | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/ReactCommon/utils/ManagedObjectWrapper.h b/ReactCommon/utils/ManagedObjectWrapper.h index 005638d087..6231580157 100644 --- a/ReactCommon/utils/ManagedObjectWrapper.h +++ b/ReactCommon/utils/ManagedObjectWrapper.h @@ -27,18 +27,7 @@ namespace detail { * A custom deleter used for the deallocation of Objective-C managed objects. * To be used only by `wrapManagedObject`. */ -static void wrappedManagedObjectDeleter(void *cfPointer) noexcept -{ - // A shared pointer does call custom deleter on `nullptr`s. - // This is somewhat counter-intuitively but makes sense considering the type-erasured nature of shared pointer and an - // aliasing constructor feature. `CFRelease` crashes on null pointer though. Therefore we must check for this case - // explicitly. - if (cfPointer == NULL) { - return; - } - - CFRelease(cfPointer); -} +void wrappedManagedObjectDeleter(void *cfPointer) noexcept; } diff --git a/ReactCommon/utils/ManagedObjectWrapper.mm b/ReactCommon/utils/ManagedObjectWrapper.mm index 1a6822eca6..ede1d30aad 100644 --- a/ReactCommon/utils/ManagedObjectWrapper.mm +++ b/ReactCommon/utils/ManagedObjectWrapper.mm @@ -10,6 +10,27 @@ #if defined(__OBJC__) && defined(__cplusplus) #if TARGET_OS_MAC && TARGET_OS_IPHONE +namespace facebook { +namespace react { +namespace detail { + +void wrappedManagedObjectDeleter(void *cfPointer) noexcept +{ + // A shared pointer does call custom deleter on `nullptr`s. + // This is somewhat counter-intuitively but makes sense considering the type-erasured nature of shared pointer and an + // aliasing constructor feature. `CFRelease` crashes on null pointer though. Therefore we must check for this case + // explicitly. + if (cfPointer == NULL) { + return; + } + + CFRelease(cfPointer); +} + +} // namespace detail +} // namespace react +} // namespace facebook + @implementation RCTInternalGenericWeakWrapper @end From b1e12fb0d57f99f80cbf93d18a6c3ee3bba0b4d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= Date: Wed, 24 Jun 2020 12:34:24 -0700 Subject: [PATCH 09/49] codegen: Move getSafePropertyName to utils Summary: Move to Utils for reuse, and change parameter to take a property object. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22146669 fbshipit-source-id: e028821cdb11361a45226de0247aa4551b5ade1d --- .../modules/ObjCppUtils/GenerateStructs.js | 59 ++++++++----------- .../generators/modules/ObjCppUtils/Utils.js | 9 +++ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js index a90d98a3be..ba0ce6a518 100644 --- a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js +++ b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js @@ -11,7 +11,11 @@ 'use strict'; import type {ObjectParamTypeAnnotation} from '../../../CodegenSchema'; -const {flatObjects, capitalizeFirstLetter} = require('./Utils'); +const { + flatObjects, + capitalizeFirstLetter, + getSafePropertyName, +} = require('./Utils'); const {generateStructsForConstants} = require('./GenerateStructsForConstants'); const template = ` @@ -44,16 +48,12 @@ inline ::_RETURN_TYPE_::JS::Native::_MODULE_NAME_::::Spec::_STRUCT_NAME_::::::_P } `; -function getSafePropertyName(name: string) { - if (name === 'id') { - return `${name}_`; - } - return name; -} - -function getNamespacedStructName(structName: string, propertyName: string) { +function getNamespacedStructName( + structName: string, + property: ObjectParamTypeAnnotation, +) { return `JS::Native::_MODULE_NAME_::::Spec${structName}${capitalizeFirstLetter( - getSafePropertyName(propertyName), + getSafePropertyName(property), )}`; } @@ -91,7 +91,7 @@ function getElementTypeForArray( case 'Int32TypeAnnotation': return 'double'; case 'ObjectTypeAnnotation': - return getNamespacedStructName(name, property.name) + 'Element'; + return getNamespacedStructName(name, property) + 'Element'; case 'GenericObjectTypeAnnotation': // TODO(T67565166): Generic objects are not type safe and should be disallowed in the schema. This case should throw an error once it is disallowed in schema. console.error( @@ -131,41 +131,40 @@ function getInlineMethodSignature( console.error( `Warning: Unsafe type found (see '${property.name}' in ${moduleName}'s ${name})`, ); - return `id ${getSafePropertyName(property.name)}() const;`; + return `id ${getSafePropertyName(property)}() const;`; } switch (typeAnnotation.type) { case 'ReservedFunctionValueTypeAnnotation': switch (typeAnnotation.name) { case 'RootTag': - return `double ${getSafePropertyName(property.name)}() const;`; + return `double ${getSafePropertyName(property)}() const;`; default: (typeAnnotation.name: empty); throw new Error(`Unknown prop type, found: ${typeAnnotation.name}"`); } case 'StringTypeAnnotation': - return `NSString *${getSafePropertyName(property.name)}() const;`; + return `NSString *${getSafePropertyName(property)}() const;`; case 'NumberTypeAnnotation': case 'FloatTypeAnnotation': case 'Int32TypeAnnotation': return `${markOptionalTypeIfNecessary('double')} ${getSafePropertyName( - property.name, + property, )}() const;`; case 'BooleanTypeAnnotation': return `${markOptionalTypeIfNecessary('bool')} ${getSafePropertyName( - property.name, + property, )}() const;`; case 'ObjectTypeAnnotation': return ( - markOptionalTypeIfNecessary( - getNamespacedStructName(name, property.name), - ) + ` ${getSafePropertyName(property.name)}() const;` + markOptionalTypeIfNecessary(getNamespacedStructName(name, property)) + + ` ${getSafePropertyName(property)}() const;` ); case 'GenericObjectTypeAnnotation': case 'AnyTypeAnnotation': return `id ${ property.optional ? '_Nullable ' : ' ' - }${getSafePropertyName(property.name)}() const;`; + }${getSafePropertyName(property)}() const;`; case 'ArrayTypeAnnotation': return `${markOptionalTypeIfNecessary( `facebook::react::LazyVector<${getElementTypeForArray( @@ -173,7 +172,7 @@ function getInlineMethodSignature( name, moduleName, )}>`, - )} ${getSafePropertyName(property.name)}() const;`; + )} ${getSafePropertyName(property)}() const;`; case 'FunctionTypeAnnotation': default: throw new Error(`Unknown prop type, found: ${typeAnnotation.type}"`); @@ -228,10 +227,7 @@ function getInlineMethodImplementation( case 'BooleanTypeAnnotation': return `RCTBridgingToBool(${element})`; case 'ObjectTypeAnnotation': - return `${getNamespacedStructName( - name, - property.name, - )}Element(${element})`; + return `${getNamespacedStructName(name, property)}Element(${element})`; case 'GenericObjectTypeAnnotation': return element; case 'AnyObjectTypeAnnotation': @@ -305,18 +301,16 @@ function getInlineMethodImplementation( return inlineTemplate .replace( /::_RETURN_TYPE_::/, - markOptionalTypeIfNecessary( - getNamespacedStructName(name, property.name), - ), + markOptionalTypeIfNecessary(getNamespacedStructName(name, property)), ) .replace( /::_RETURN_VALUE_::/, property.optional ? `(p == nil ? folly::none : folly::make_optional(${getNamespacedStructName( name, - property.name, + property, )}(p)))` - : `${getNamespacedStructName(name, property.name)}(p)`, + : `${getNamespacedStructName(name, property)}(p)`, ); case 'ArrayTypeAnnotation': return inlineTemplate @@ -366,10 +360,7 @@ function translateObjectsForStructs( acc.concat( object.properties.map(property => getInlineMethodImplementation(property, object.name, moduleName) - .replace( - /::_PROPERTY_NAME_::/g, - getSafePropertyName(property.name), - ) + .replace(/::_PROPERTY_NAME_::/g, getSafePropertyName(property)) .replace(/::_STRUCT_NAME_::/g, object.name), ), ), diff --git a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/Utils.js b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/Utils.js index df7bff1abb..eb7d73974e 100644 --- a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/Utils.js +++ b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/Utils.js @@ -86,7 +86,16 @@ function flatObjects( return flattenObjects; } + +function getSafePropertyName(property: ObjectParamTypeAnnotation): string { + if (property.name === 'id') { + return `${property.name}_`; + } + return property.name; +} + module.exports = { flatObjects, capitalizeFirstLetter, + getSafePropertyName, }; From 2e27fb6a9776463fa865197db652ec3016b97b8d Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 24 Jun 2020 16:34:24 -0700 Subject: [PATCH 10/49] LayoutAnimations: for all Transform matrices, store decomposed matrix and use that for interpolation Summary: Previously, we tried to take a Transform matrix, decompose it into parts, and then interpolate between those parts. This will always be risky at best, and in some cases ambiguous or unsolvable. For example, a scale of -1 is identical to a rotation of 180 degrees. Another issue is that when decomposing a matrix, it is impossible to tell the sign of scaleX, scaleY, scaleZ. This is a problem - flipping a View over an axis via scale then becomes a non-animatable operation. This caused a number of issues. To resolve it, we accumulate the "operations" resulting in a particular transform. This allows us to easily interpolate between two Transform matrices without actually decomposing the matrix, since we have the "path" that resulted in each particular matrix. This will make LayoutAnimations over transforms, including Skew transforms, look and work much better, and more reliably. Changelog: [Internal] Reviewed By: shergin Differential Revision: D22204559 fbshipit-source-id: 0d88ae77e4399a7ea333afbf6062beea977b854a --- .../fabric/components/view/conversions.h | 2 + ReactCommon/fabric/graphics/Quaternion.h | 208 ------------ ReactCommon/fabric/graphics/Transform.cpp | 303 ++++++++++-------- ReactCommon/fabric/graphics/Transform.h | 64 ++-- 4 files changed, 199 insertions(+), 378 deletions(-) delete mode 100644 ReactCommon/fabric/graphics/Quaternion.h diff --git a/ReactCommon/fabric/components/view/conversions.h b/ReactCommon/fabric/components/view/conversions.h index 0ac41614dd..5a3cb1a8cc 100644 --- a/ReactCommon/fabric/components/view/conversions.h +++ b/ReactCommon/fabric/components/view/conversions.h @@ -422,6 +422,8 @@ inline void fromRawValue(const RawValue &value, Transform &result) { for (auto number : numbers) { transformMatrix.matrix[i++] = number; } + transformMatrix.operations.push_back( + TransformOperation{TransformOperationType::Arbitrary, 0, 0, 0}); } else if (operation == "perspective") { transformMatrix = transformMatrix * Transform::Perspective((Float)parameters); diff --git a/ReactCommon/fabric/graphics/Quaternion.h b/ReactCommon/fabric/graphics/Quaternion.h deleted file mode 100644 index 4233a2c6cb..0000000000 --- a/ReactCommon/fabric/graphics/Quaternion.h +++ /dev/null @@ -1,208 +0,0 @@ -/* - * Portions Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -#pragma once - -#include -#include -#include - -// The following is a modified, stripped-down version of the Quaternion class -// by Frank Astier. Copyright notice below. -// The original has many, many more features, and has been stripped down -// to support the exact data-structures and use-cases we need for React Native. - -/** - * The MIT License (MIT) - * - * Copyright (c) 2015 Frank Astier - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -namespace facebook { -namespace react { - -template -class Quaternion { - public: - /** - * Copy constructor. - */ - Quaternion(const Quaternion &y) : a_(y.a_), b_(y.b_), c_(y.c_), d_(y.d_) {} - - Quaternion(T a, T b, T c, T d) : a_(a), b_(b), c_(c), d_(d) {} - - static Quaternion fromRotationMatrix(std::array const &rm) { - T t = rm[0 * 4 + 0] + rm[1 * 4 + 1] + rm[2 * 4 + 2]; - if (t > 0) { - T s = (T)0.5 / std::sqrt(t + 1); - return {(T)0.25 / s, - (rm[2 * 4 + 1] - rm[1 * 4 + 2]) * s, - (rm[0 * 4 + 2] - rm[2 * 4 + 0]) * s, - (rm[1 * 4 + 0] - rm[0 * 4 + 1]) * s}; - } else if (rm[0 * 4 + 0] > rm[1 * 4 + 1] && rm[0 * 4 + 0] > rm[2 * 4 + 2]) { - T s = (T)2.0 * - std::sqrt( - 1.0 + rm[0 * 4 + 0] - rm[1 * 4 + 1] - rm[2 * 4 + 2]); // S=4*qx - return {(rm[2 * 4 + 1] - rm[1 * 4 + 2]) / s, - (T)0.25 * s, - (rm[0 * 4 + 1] + rm[1 * 4 + 0]) / s, - (rm[0 * 4 + 2] + rm[2 * 4 + 0]) / s}; - } else if (rm[1 * 4 + 1] > rm[2 * 4 + 2]) { - T s = (T)2.0 * - std::sqrt( - 1.0 + rm[1 * 4 + 1] - rm[0 * 4 + 0] - rm[2 * 4 + 2]); // S=4*qy - return {(rm[0 * 4 + 2] - rm[2 * 4 + 0]) / s, - (rm[0 * 4 + 1] + rm[1 * 4 + 0]) / s, - (T)0.25 * s, - (rm[1 * 4 + 2] + rm[2 * 4 + 1]) / s}; - } else { - T s = (T)2.0 * - std::sqrt( - 1.0 + rm[2 * 4 + 2] - rm[0 * 4 + 0] - rm[1 * 4 + 1]); // S=4*qz - return {(rm[1 * 4 + 0] - rm[0 * 4 + 1]) / s, - (rm[0 * 4 + 2] + rm[2 * 4 + 0]) / s, - (rm[1 * 4 + 2] + rm[2 * 4 + 1]) / s, - (T)0.25 * s}; - } - } - - /** - * Returns a 3D, 4x4 rotation matrix. - * This is the "homogeneous" expression to convert to a rotation matrix, - * which works even when the Quaternion is not a unit Quaternion. - */ - inline std::array toRotationMatrix4x4() { - T a2 = a_ * a_, b2 = b_ * b_, c2 = c_ * c_, d2 = d_ * d_; - T ab = a_ * b_, ac = a_ * c_, ad = a_ * d_; - T bc = b_ * c_, bd = b_ * d_; - T cd = c_ * d_; - return {a2 + b2 - c2 - d2, - 2 * (bc - ad), - 2 * (bd + ac), - 0, - 2 * (bc + ad), - a2 - b2 + c2 - d2, - 2 * (cd - ab), - 0, - 2 * (bd - ac), - 2 * (cd + ab), - a2 - b2 - c2 + d2, - 0, - 0, - 0, - 0, - 1}; - } - - inline Quaternion normalize() const { - assert(abs() > 0); // or this is not normalizable - T factor = abs(); - return *this / (factor != 0 ? factor : 1); - } - - inline T dot(const Quaternion &other) { - return a_ * other.a_ + b_ * other.b_ + c_ * other.c_ + d_ * other.d_; - } - - /** - * The square of the norm of the Quaternion. - * (The square is sometimes useful, and it avoids paying for a sqrt). - */ - inline T norm_squared() const { - return a_ * a_ + b_ * b_ + c_ * c_ + d_ * d_; - } - - /** - * The norm of the Quaternion (the l2 norm). - */ - inline T abs() const { - return std::sqrt(norm_squared()); - } - - inline Quaternion operator/=(T y) { - a_ /= y; - b_ /= y; - c_ /= y; - d_ /= y; - return *this; - } - - inline Quaternion operator*=(T y) { - a_ *= y; - b_ *= y; - c_ *= y; - d_ *= y; - return *this; - } - - inline Quaternion operator+=(Quaternion const &other) { - a_ += other.a_; - b_ += other.b_; - c_ += other.c_; - d_ += other.d_; - return *this; - } - - inline Quaternion operator-=(Quaternion const &other) { - a_ -= other.a_; - b_ -= other.b_; - c_ -= other.c_; - d_ -= other.d_; - return *this; - } - - private: - T a_; // AKA w, qw - T b_; // AKA x, qx - T c_; // AKA y, qy - T d_; // AKA z, qz -}; - -template -inline Quaternion operator/(Quaternion const &lhs, T rhs) { - return Quaternion(lhs) /= rhs; -} - -template -inline Quaternion operator*(Quaternion const &lhs, T rhs) { - return Quaternion(lhs) *= rhs; -} - -template -inline Quaternion operator+( - Quaternion const &lhs, - Quaternion const &rhs) { - return Quaternion(lhs) += rhs; -} - -template -inline Quaternion operator-( - Quaternion const &lhs, - Quaternion const &rhs) { - return Quaternion(lhs) -= rhs; -} - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/fabric/graphics/Transform.cpp b/ReactCommon/fabric/graphics/Transform.cpp index f1711f0418..6caea8e974 100644 --- a/ReactCommon/fabric/graphics/Transform.cpp +++ b/ReactCommon/fabric/graphics/Transform.cpp @@ -7,7 +7,6 @@ #include "Transform.h" -#include #include #include @@ -34,203 +33,211 @@ Transform Transform::Identity() { Transform Transform::Perspective(Float perspective) { auto transform = Transform{}; + transform.operations.push_back(TransformOperation{ + TransformOperationType::Perspective, perspective, 0, 0}); transform.matrix[11] = -1 / perspective; return transform; } -Transform Transform::Scale(Float factorX, Float factorY, Float factorZ) { +Transform Transform::Scale(Float x, Float y, Float z) { auto transform = Transform{}; - transform.matrix[0] = factorX; - transform.matrix[5] = factorY; - transform.matrix[10] = factorZ; + Float xprime = isZero(x) ? 0 : x; + Float yprime = isZero(y) ? 0 : y; + Float zprime = isZero(z) ? 0 : z; + if (xprime != 1 || yprime != 1 || zprime != 1) { + transform.operations.push_back(TransformOperation{ + TransformOperationType::Scale, xprime, yprime, zprime}); + transform.matrix[0] = xprime; + transform.matrix[5] = yprime; + transform.matrix[10] = zprime; + } return transform; } Transform Transform::Translate(Float x, Float y, Float z) { auto transform = Transform{}; - transform.matrix[12] = x; - transform.matrix[13] = y; - transform.matrix[14] = z; + Float xprime = isZero(x) ? 0 : x; + Float yprime = isZero(y) ? 0 : y; + Float zprime = isZero(z) ? 0 : z; + if (xprime != 0 || yprime != 0 || zprime != 0) { + transform.operations.push_back(TransformOperation{ + TransformOperationType::Translate, xprime, yprime, zprime}); + transform.matrix[12] = xprime; + transform.matrix[13] = yprime; + transform.matrix[14] = zprime; + } return transform; } Transform Transform::Skew(Float x, Float y) { auto transform = Transform{}; - transform.matrix[4] = std::tan(x); - transform.matrix[1] = std::tan(y); + Float xprime = isZero(x) ? 0 : x; + Float yprime = isZero(y) ? 0 : y; + transform.operations.push_back( + TransformOperation{TransformOperationType::Skew, xprime, yprime, 0}); + transform.matrix[4] = std::tan(xprime); + transform.matrix[1] = std::tan(yprime); return transform; } Transform Transform::RotateX(Float radians) { auto transform = Transform{}; - transform.matrix[5] = std::cos(radians); - transform.matrix[6] = std::sin(radians); - transform.matrix[9] = -std::sin(radians); - transform.matrix[10] = std::cos(radians); + if (!isZero(radians)) { + transform.operations.push_back( + TransformOperation{TransformOperationType::Rotate, radians, 0, 0}); + transform.matrix[5] = std::cos(radians); + transform.matrix[6] = std::sin(radians); + transform.matrix[9] = -std::sin(radians); + transform.matrix[10] = std::cos(radians); + } return transform; } Transform Transform::RotateY(Float radians) { auto transform = Transform{}; - transform.matrix[0] = std::cos(radians); - transform.matrix[2] = -std::sin(radians); - transform.matrix[8] = std::sin(radians); - transform.matrix[10] = std::cos(radians); + if (!isZero(radians)) { + transform.operations.push_back( + TransformOperation{TransformOperationType::Rotate, 0, radians, 0}); + transform.matrix[0] = std::cos(radians); + transform.matrix[2] = -std::sin(radians); + transform.matrix[8] = std::sin(radians); + transform.matrix[10] = std::cos(radians); + } return transform; } Transform Transform::RotateZ(Float radians) { auto transform = Transform{}; - transform.matrix[0] = std::cos(radians); - transform.matrix[1] = std::sin(radians); - transform.matrix[4] = -std::sin(radians); - transform.matrix[5] = std::cos(radians); + if (!isZero(radians)) { + transform.operations.push_back( + TransformOperation{TransformOperationType::Rotate, 0, 0, radians}); + transform.matrix[0] = std::cos(radians); + transform.matrix[1] = std::sin(radians); + transform.matrix[4] = -std::sin(radians); + transform.matrix[5] = std::cos(radians); + } return transform; } Transform Transform::Rotate(Float x, Float y, Float z) { auto transform = Transform{}; - if (x != 0) { + transform.operations.push_back( + TransformOperation{TransformOperationType::Rotate, x, y, z}); + if (!isZero(x)) { transform = transform * Transform::RotateX(x); } - if (y != 0) { + if (!isZero(y)) { transform = transform * Transform::RotateY(y); } - if (z != 0) { + if (!isZero(z)) { transform = transform * Transform::RotateZ(z); } return transform; } -Transform::SRT Transform::ExtractSRT(Transform const &t) { - // First we need to extract translation, rotation, and scale from both - // matrices, in that order. Matrices must be in this form: [a b c d] [e f g h] - // [i j k l] - // [0 0 0 1] - // We also assume that all scale factors are non-negative. - // TODO T68587989: If ViewProps retains the underlying transform props instead - // of just the matrix version of transforms, then we can use those properties - // directly instead of decomposing properties from a matrix which will always - // be lossy. Because of these assumptions, animations involving negative - // scale/rotation and anything involving skews will not look great. - // assert( - // t.matrix[12] == 0 && t.matrix[13] == 0 && t.matrix[14] == 0 && - // t.matrix[15] == 1 && "Last row of matrix must be [0,0,0,1]"); +Transform Transform::FromTransformOperation( + TransformOperation transformOperation) { + if (transformOperation.type == TransformOperationType::Perspective) { + return Transform::Perspective(transformOperation.x); + } + if (transformOperation.type == TransformOperationType::Scale) { + return Transform::Scale( + transformOperation.x, transformOperation.y, transformOperation.z); + } + if (transformOperation.type == TransformOperationType::Translate) { + return Transform::Translate( + transformOperation.x, transformOperation.y, transformOperation.z); + } + if (transformOperation.type == TransformOperationType::Skew) { + return Transform::Skew(transformOperation.x, transformOperation.y); + } + if (transformOperation.type == TransformOperationType::Rotate) { + return Transform::Rotate( + transformOperation.x, transformOperation.y, transformOperation.z); + } - // lhs: - // Translation: extract the values from the rightmost column - Float translationX = t.matrix[3]; - Float translationY = t.matrix[7]; - Float translationZ = t.matrix[11]; + // Identity or Arbitrary + return Transform::Identity(); +} - // Scale: the length of the first three column vectors - // TODO: do we need to do anything special for negative scale factors? - // the last element is a uniform scale factor - Float scaleX = t.matrix[15] * - sqrt(pow(t.matrix[0], 2) + pow(t.matrix[4], 2) + - pow(t.matrix[8], 2)); // sqrt(a^2 + e^2 + i^2) - Float scaleY = t.matrix[15] * - sqrt(pow(t.matrix[1], 2) + pow(t.matrix[5], 2) + - pow(t.matrix[9], 2)); // sqrt(b^2 + f^2 + j^2) - Float scaleZ = t.matrix[15] * - sqrt(pow(t.matrix[2], 2) + pow(t.matrix[6], 2) + - pow(t.matrix[10], 2)); // sqrt(c^2 + g^2 + k^2) - - Float rScaleFactorX = scaleX == 0 ? 1 : scaleX; - Float rScaleFactorY = scaleY == 0 ? 1 : scaleY; - Float rScaleFactorZ = scaleZ == 0 ? 1 : scaleZ; - - // Construct a rotation matrix and convert that to quaternions - auto rotationMatrix = std::array{t.matrix[0] / rScaleFactorX, - t.matrix[1] / rScaleFactorY, - t.matrix[2] / rScaleFactorZ, - 0, - t.matrix[4] / rScaleFactorX, - t.matrix[5] / rScaleFactorY, - t.matrix[6] / rScaleFactorZ, - 0, - t.matrix[8] / rScaleFactorX, - t.matrix[9] / rScaleFactorY, - t.matrix[10] / rScaleFactorZ, - 0, - 0, - 0, - 0, - 1}; - - Quaternion q = - Quaternion::fromRotationMatrix(rotationMatrix).normalize(); - - return Transform::SRT{ - translationX, translationY, translationZ, scaleX, scaleY, scaleZ, q}; +TransformOperation Transform::DefaultTransformOperation( + TransformOperationType type) { + switch (type) { + case TransformOperationType::Arbitrary: + return TransformOperation{TransformOperationType::Arbitrary, 0, 0, 0}; + case TransformOperationType::Perspective: + return TransformOperation{TransformOperationType::Perspective, 0, 0, 0}; + case TransformOperationType::Scale: + return TransformOperation{TransformOperationType::Scale, 1, 1, 1}; + case TransformOperationType::Translate: + return TransformOperation{TransformOperationType::Translate, 0, 0, 0}; + case TransformOperationType::Rotate: + return TransformOperation{TransformOperationType::Rotate, 0, 0, 0}; + case TransformOperationType::Skew: + return TransformOperation{TransformOperationType::Skew, 0, 0, 0}; + default: + case TransformOperationType::Identity: + return TransformOperation{TransformOperationType::Identity, 0, 0, 0}; + } } Transform Transform::Interpolate( float animationProgress, Transform const &lhs, Transform const &rhs) { - // Extract SRT for both sides - // This is extracted in the form: X,Y,Z coordinates for translations; X,Y,Z - // coordinates for scale; and a quaternion for rotation. - auto lhsSRT = ExtractSRT(lhs); - auto rhsSRT = ExtractSRT(rhs); + // Iterate through operations and reconstruct an interpolated resulting + // transform If at any point we hit an "Arbitrary" Transform, return at that + // point + Transform result = Transform::Identity(); + for (int i = 0, j = 0; + i < lhs.operations.size() || j < rhs.operations.size();) { + bool haveLHS = i < lhs.operations.size(); + bool haveRHS = j < rhs.operations.size(); - // Interpolate translation and scale terms linearly (LERP) - Float translateX = - (lhsSRT.translationX + - (rhsSRT.translationX - lhsSRT.translationX) * animationProgress); - Float translateY = - (lhsSRT.translationY + - (rhsSRT.translationY - lhsSRT.translationY) * animationProgress); - Float translateZ = - (lhsSRT.translationZ + - (rhsSRT.translationZ - lhsSRT.translationZ) * animationProgress); - Float scaleX = - (lhsSRT.scaleX + (rhsSRT.scaleX - lhsSRT.scaleX) * animationProgress); - Float scaleY = - (lhsSRT.scaleY + (rhsSRT.scaleY - lhsSRT.scaleY) * animationProgress); - Float scaleZ = - (lhsSRT.scaleZ + (rhsSRT.scaleZ - lhsSRT.scaleZ) * animationProgress); + if ((haveLHS && + lhs.operations[i].type == TransformOperationType::Arbitrary) || + (haveRHS && + rhs.operations[i].type == TransformOperationType::Arbitrary)) { + return result; + } + if (haveLHS && lhs.operations[i].type == TransformOperationType::Identity) { + i++; + continue; + } + if (haveRHS && rhs.operations[j].type == TransformOperationType::Identity) { + j++; + continue; + } - // Use the quaternion vectors to produce an interpolated rotation via SLERP - // dot: cos of the angle between the two quaternion vectors - Quaternion q1 = lhsSRT.rotation; - Quaternion q2 = rhsSRT.rotation; - Float dot = q1.dot(q2); - // Clamp dot between -1 and 1 - dot = (dot < -1 ? -1 : (dot > 1 ? 1 : dot)); - // There are two ways of performing an identical slerp: q1 and -q1. - // If the dot-product is negative, we can multiply q1 by -1 and our animation - // will take the "short way" around instead of the "long way". - if (dot < 0) { - q1 = q1 * (Float)-1; - dot = dot * -1; - } - // Interpolated angle - Float theta = acosf(dot) * animationProgress; + // Here we either set: + // 1. lhs = next left op, rhs = next right op (when types are identical and + // both exist) + // 2. lhs = next left op, rhs = default of type (if types unequal, or rhs + // doesn't exist) + // 3. lhs = default of type, rhs = next right op (if types unequal, or rhs + // doesn't exist) This guarantees that the types of both sides are equal, + // and that one or both indices moves forward. + TransformOperationType type = + (haveLHS ? lhs.operations[i] : rhs.operations[j]).type; + TransformOperation lhsOp = + (haveLHS ? lhs.operations[i++] + : Transform::DefaultTransformOperation(type)); + TransformOperation rhsOp = + (haveRHS && rhs.operations[j].type == type + ? rhs.operations[j++] + : Transform::DefaultTransformOperation(type)); + assert(type == lhsOp.type); + assert(type == rhsOp.type); - Transform rotation = Transform::Identity(); - - // Compute orthonormal basis - Quaternion orthonormalBasis = (q2 - q1 * dot); - - if (orthonormalBasis.abs() > 0) { - Quaternion orthonormalBasisNormalized = orthonormalBasis.normalize(); - - // Compute orthonormal basis - // Final quaternion result - slerp! - Quaternion resultingRotationVec = - (q1 * (Float)cos(theta) + - orthonormalBasisNormalized * (Float)sin(theta)) - .normalize(); - - // Convert quaternion to matrix - rotation.matrix = resultingRotationVec.toRotationMatrix4x4(); + result = result * + Transform::FromTransformOperation(TransformOperation{ + type, + lhsOp.x + (rhsOp.x - lhsOp.x) * animationProgress, + lhsOp.y + (rhsOp.y - lhsOp.y) * animationProgress, + lhsOp.z + (rhsOp.z - lhsOp.z) * animationProgress}); } - // Compose matrices and return - return (Scale(scaleX, scaleY, scaleZ) * rotation) * - Translate(translateX, translateY, translateZ); + return result; } bool Transform::operator==(Transform const &rhs) const { @@ -253,6 +260,20 @@ Transform Transform::operator*(Transform const &rhs) const { const auto &lhs = *this; auto result = Transform{}; + for (const auto &op : this->operations) { + if (op.type == TransformOperationType::Identity && + result.operations.size() > 0) { + continue; + } + result.operations.push_back(op); + } + for (const auto &op : rhs.operations) { + if (op.type == TransformOperationType::Identity && + result.operations.size() > 0) { + continue; + } + result.operations.push_back(op); + } auto lhs00 = lhs.matrix[0], lhs01 = lhs.matrix[1], lhs02 = lhs.matrix[2], lhs03 = lhs.matrix[3], lhs10 = lhs.matrix[4], lhs11 = lhs.matrix[5], diff --git a/ReactCommon/fabric/graphics/Transform.h b/ReactCommon/fabric/graphics/Transform.h index 357e2863d7..7d14a635af 100644 --- a/ReactCommon/fabric/graphics/Transform.h +++ b/ReactCommon/fabric/graphics/Transform.h @@ -8,11 +8,11 @@ #pragma once #include +#include #include #include #include -#include #ifdef ANDROID #include @@ -21,21 +21,38 @@ namespace facebook { namespace react { -struct ScaleRotationTranslation { - Float translationX; - Float translationY; - Float translationZ; - Float scaleX; - Float scaleY; - Float scaleZ; - Quaternion rotation; +inline bool isZero(Float n) { + // We use this ternary expression instead of abs, fabsf, etc, because + // Float can be double or float depending on compilation target. + return (n < 0 ? n * (-1) : n) < 0.00001; +} + +/** + * Defines operations used to construct a transform matrix. + * An "Arbitrary" operation means that the transform was seeded with some + * arbitrary initial result. + */ +enum class TransformOperationType { + Arbitrary, + Identity, + Perspective, + Scale, + Translate, + Rotate, + Skew +}; +struct TransformOperation { + TransformOperationType type; + Float x; + Float y; + Float z; }; /* * Defines transform matrix to apply affine transformations. */ struct Transform { - using SRT = ScaleRotationTranslation; + std::vector operations{}; std::array matrix{ {1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1}}; @@ -47,6 +64,14 @@ struct Transform { static void print(Transform const &t, std::string prefix); #endif + /* + * Given a TransformOperation, return the proper transform. + */ + static Transform FromTransformOperation( + TransformOperation transformOperation); + static TransformOperation DefaultTransformOperation( + TransformOperationType type); + /* * Returns the identity transform (`[1 0 0 0; 0 1 0 0; 0 0 1 0; 0 0 0 1]`). */ @@ -80,25 +105,6 @@ struct Transform { static Transform RotateZ(Float angle); static Transform Rotate(Float angleX, Float angleY, Float angleZ); - /** - * Extract SRT (scale, rotation, transformation) from a Transform matrix. - * - * CAVEATS: - * 1. The input matrix must not have Skew applied. - * 2. Scaling factors must be non-negative. Scaling by a negative factor is - * equivalent to a rotation, and though it is possible to detect if 1 or - * 3 of the scale signs are flipped (but not two), it is not possible - * to detect WHICH of the scales are flipped. Thus, any animation - * that involves a negative scale factor will not crash but will - * interpolate over nonsensical values. - * 3. Another caveat is that if the animation interpolates TO a 90º - * rotation in the X, Y, or Z axis, the View will appear to suddenly - * explode in size. Interpolating THROUGH 90º is fine as long as you don't end - * up at 90º or close to it (89.99). The same is true for 0±90 and 360n+90, - * etc. - */ - static SRT ExtractSRT(Transform const &transform); - /** * Perform an interpolation between lhs and rhs, given progress. * This first decomposes the matrices into translation, scale, and rotation, From 6342e6e3f11219391ac3296d41233735af7e6cad Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 24 Jun 2020 16:34:24 -0700 Subject: [PATCH 11/49] LayoutAnimations: attempt to work around potential crashes when animation is racing with setup or teardown Summary: Attempt to fix T68951888 where (1) ComponentHandle is zero, and/or (2) ComponentHandle is missing in the registry. Either will cause a crash and both should be trivial to work around. I was able to repro once accidentally in about 1/200 sessions, so I am not 100% sure if this fixes the root cause. Changelog: [Internal] Reviewed By: shergin Differential Revision: D22216030 fbshipit-source-id: b6986adee6fe739ce58579a2b031a2d252e73e35 --- .../LayoutAnimationKeyFrameManager.cpp | 39 ++++++++++++++----- .../LayoutAnimationKeyFrameManager.h | 1 + .../ComponentDescriptorRegistry.cpp | 12 ++++++ .../ComponentDescriptorRegistry.h | 2 + 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp index ae8c6b13d3..9d97fb1d53 100644 --- a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp +++ b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.cpp @@ -521,8 +521,8 @@ LayoutAnimationKeyFrameManager::pullTransaction( mutation.type == ShadowViewMutation::Type::Remove ? mutation.oldChildShadowView : mutation.newChildShadowView); - auto const &componentDescriptor = - getComponentDescriptorForShadowView(baselineShadowView); + bool haveComponentDescriptor = + hasComponentDescriptorForShadowView(baselineShadowView); auto mutationConfig = (mutation.type == ShadowViewMutation::Type::Delete @@ -660,8 +660,11 @@ LayoutAnimationKeyFrameManager::pullTransaction( AnimationKeyFrame keyFrame{}; if (mutation.type == ShadowViewMutation::Type::Insert) { if (mutationConfig->animationProperty == - AnimationProperty::Opacity) { - auto props = componentDescriptor.cloneProps(viewStart.props, {}); + AnimationProperty::Opacity && + haveComponentDescriptor) { + auto props = + getComponentDescriptorForShadowView(baselineShadowView) + .cloneProps(viewStart.props, {}); const auto viewProps = dynamic_cast(props.get()); if (viewProps != nullptr) { @@ -675,8 +678,10 @@ LayoutAnimationKeyFrameManager::pullTransaction( bool isScaleY = mutationConfig->animationProperty == AnimationProperty::ScaleY || mutationConfig->animationProperty == AnimationProperty::ScaleXY; - if (isScaleX || isScaleY) { - auto props = componentDescriptor.cloneProps(viewStart.props, {}); + if ((isScaleX || isScaleY) && haveComponentDescriptor) { + auto props = + getComponentDescriptorForShadowView(baselineShadowView) + .cloneProps(viewStart.props, {}); const auto viewProps = dynamic_cast(props.get()); if (viewProps != nullptr) { @@ -695,8 +700,11 @@ LayoutAnimationKeyFrameManager::pullTransaction( 0}; } else if (mutation.type == ShadowViewMutation::Type::Delete) { if (mutationConfig->animationProperty == - AnimationProperty::Opacity) { - auto props = componentDescriptor.cloneProps(viewFinal.props, {}); + AnimationProperty::Opacity && + haveComponentDescriptor) { + auto props = + getComponentDescriptorForShadowView(baselineShadowView) + .cloneProps(viewFinal.props, {}); const auto viewProps = dynamic_cast(props.get()); if (viewProps != nullptr) { @@ -710,8 +718,10 @@ LayoutAnimationKeyFrameManager::pullTransaction( bool isScaleY = mutationConfig->animationProperty == AnimationProperty::ScaleY || mutationConfig->animationProperty == AnimationProperty::ScaleXY; - if (isScaleX || isScaleY) { - auto props = componentDescriptor.cloneProps(viewFinal.props, {}); + if ((isScaleX || isScaleY) && haveComponentDescriptor) { + auto props = + getComponentDescriptorForShadowView(baselineShadowView) + .cloneProps(viewFinal.props, {}); const auto viewProps = dynamic_cast(props.get()); if (viewProps != nullptr) { @@ -983,6 +993,12 @@ bool LayoutAnimationKeyFrameManager::mutatedViewIsVirtual( return viewIsVirtual; } +bool LayoutAnimationKeyFrameManager::hasComponentDescriptorForShadowView( + ShadowView const &shadowView) const { + return componentDescriptorRegistry_->hasComponentDescriptorAt( + shadowView.componentHandle); +} + ComponentDescriptor const & LayoutAnimationKeyFrameManager::getComponentDescriptorForShadowView( ShadowView const &shadowView) const { @@ -1008,6 +1024,9 @@ ShadowView LayoutAnimationKeyFrameManager::createInterpolatedShadowView( AnimationConfig const &animationConfig, ShadowView startingView, ShadowView finalView) const { + if (!hasComponentDescriptorForShadowView(startingView)) { + return finalView; + } ComponentDescriptor const &componentDescriptor = getComponentDescriptorForShadowView(startingView); auto mutatedShadowView = ShadowView(startingView); diff --git a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.h b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.h index 9ce662a51f..73906fb65e 100644 --- a/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.h +++ b/ReactCommon/fabric/animations/LayoutAnimationKeyFrameManager.h @@ -148,6 +148,7 @@ class LayoutAnimationKeyFrameManager : public UIManagerAnimationDelegate, protected: bool mutatedViewIsVirtual(ShadowViewMutation const &mutation) const; + bool hasComponentDescriptorForShadowView(ShadowView const &shadowView) const; ComponentDescriptor const &getComponentDescriptorForShadowView( ShadowView const &shadowView) const; std::pair calculateAnimationProgress( diff --git a/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.cpp b/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.cpp index b85cf63661..2081886c66 100644 --- a/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.cpp +++ b/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.cpp @@ -168,6 +168,18 @@ ComponentDescriptor const &ComponentDescriptorRegistry::at( return *_registryByHandle.at(componentHandle); } +bool ComponentDescriptorRegistry::hasComponentDescriptorAt( + ComponentHandle componentHandle) const { + std::shared_lock lock(mutex_); + + auto iterator = _registryByHandle.find(componentHandle); + if (iterator == _registryByHandle.end()) { + return false; + } + + return true; +} + SharedShadowNode ComponentDescriptorRegistry::createNode( Tag tag, std::string const &viewName, diff --git a/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.h b/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.h index 3f25839bf5..07e211d7d0 100644 --- a/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.h +++ b/ReactCommon/fabric/componentregistry/ComponentDescriptorRegistry.h @@ -51,6 +51,8 @@ class ComponentDescriptorRegistry { ComponentDescriptor const &at(std::string const &componentName) const; ComponentDescriptor const &at(ComponentHandle componentHandle) const; + bool hasComponentDescriptorAt(ComponentHandle componentHandle) const; + ShadowNode::Shared createNode( Tag tag, std::string const &viewName, From 37e8cd0b46cdc9d6aaff416f1eb6f9a3b1dba8c3 Mon Sep 17 00:00:00 2001 From: Omer Peleg Date: Thu, 25 Jun 2020 07:02:14 -0700 Subject: [PATCH 12/49] Move powermock config target to xplat and make fb_java_library non-fbandroid-specific Summary: Changelog: [Internal] Reviewed By: brianduff Differential Revision: D22207494 fbshipit-source-id: c3a87662cb6330a4ab0801c9ee48c0d5723d324a --- tools/build_defs/oss/rn_defs.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build_defs/oss/rn_defs.bzl b/tools/build_defs/oss/rn_defs.bzl index 887e25d365..5553eb884a 100644 --- a/tools/build_defs/oss/rn_defs.bzl +++ b/tools/build_defs/oss/rn_defs.bzl @@ -188,7 +188,7 @@ def rn_robolectric_test(name, srcs, vm_args = None, *args, **kwargs): kwargs["deps"] = kwargs.pop("deps", []) + [ react_native_android_toplevel_dep("third-party/java/mockito2:mockito2"), - react_native_dep("libraries/fbcore/src/test/java/com/facebook/powermock:powermock2"), + react_native_xplat_dep("libraries/fbcore/src/test/java/com/facebook/powermock:powermock2"), react_native_dep("third-party/java/robolectric/4.3.1:robolectric"), ] From 530d11c50626cbf23d34345b8c51983fcff9e919 Mon Sep 17 00:00:00 2001 From: Omer Peleg Date: Thu, 25 Jun 2020 10:49:57 -0700 Subject: [PATCH 13/49] Fix RN OSS build (#29221) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/29221 Changelog: [Internal] Reviewed By: GijsWeterings Differential Revision: D22234372 fbshipit-source-id: 9b83ec3aa57254826be44ac0f73aeb34fb859337 --- .../libraries/fbcore/src/test/java/com/facebook/powermock/BUCK | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {ReactAndroid/src/main => ReactCommon}/libraries/fbcore/src/test/java/com/facebook/powermock/BUCK (100%) diff --git a/ReactAndroid/src/main/libraries/fbcore/src/test/java/com/facebook/powermock/BUCK b/ReactCommon/libraries/fbcore/src/test/java/com/facebook/powermock/BUCK similarity index 100% rename from ReactAndroid/src/main/libraries/fbcore/src/test/java/com/facebook/powermock/BUCK rename to ReactCommon/libraries/fbcore/src/test/java/com/facebook/powermock/BUCK From e75557b48fbee1d136b8b7d1a78ea8f9b9467479 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 25 Jun 2020 15:29:10 -0700 Subject: [PATCH 14/49] Restore Previous Behavior for StyleSheet Validation of Null/Undefined Styles (#29171) Summary: https://github.com/facebook/react-native/issues/27264 changed stylesheet validation to avoid enumerating properties on the prototype of a style. It introduces a secondary behavior change, where null/undefined styles used to be tolerated but now lead to an exception. This is because `for in undefined` will noop where `for of Object.keys(undefined)` will throw. This scenario of undefined/null styles seems to actually show up in practice and was previously well tolerated. E.g. `Button.js` has code that looks like this: ```jsx const styles = StyleSheet.create({ button: Platform.select({ ios: {}, android: { elevation: 4, // Material design blue from https://material.google.com/style/color.html#color-color-palette backgroundColor: '#2196F3', borderRadius: 2, }, }), ``` For non ios/Android platforms, that creates a style object which looks like: ```js { button: undefined, ... } ``` This previously meant that the component would be unstyled if created, but now means out-of-tree platforms throw if the builtin Button component is required. This change restores the previous `for in` loop but adds a `hasOwnProperty` check to avoid properties on prototypes. ## Changelog [General] [Fixed] - Restore Previous Behavior for StyleSheet Validation of Null/Undefined Styles Pull Request resolved: https://github.com/facebook/react-native/pull/29171 Test Plan: Validated that importing Buttons will no longer cause an exception, and that invalid properties are still caught. Reviewed By: JoshuaGross Differential Revision: D22118379 Pulled By: TheSavior fbshipit-source-id: 650c64b934ccd12a3dc1b75e95debc359925ad73 --- Libraries/StyleSheet/StyleSheetValidation.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Libraries/StyleSheet/StyleSheetValidation.js b/Libraries/StyleSheet/StyleSheetValidation.js index bfa09f702e..456953f7a0 100644 --- a/Libraries/StyleSheet/StyleSheetValidation.js +++ b/Libraries/StyleSheet/StyleSheetValidation.js @@ -51,6 +51,9 @@ class StyleSheetValidation { if (!__DEV__ || global.__RCTProfileIsProfiling) { return; } + if (!styles[name]) { + return; + } const styleProps = Object.keys(styles[name]); for (const prop of styleProps) { StyleSheetValidation.validateStyleProp( From 87a2e29f5928c2e09ac9a98c54732d5f697d8e61 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Sat, 27 Jun 2020 02:15:58 -0700 Subject: [PATCH 15/49] EventEmitter: Delete `once()` and `removeCurrentListener()` Summary: In an effort to simplify and clean up the `EventEmitter` abstractions in React Native, this removes `once()` and `removeCurrentListener()`. Across the Facebook codebase, there were only two callers of `once()` and no callers of `removeCurrentListener()`. The same behavior can be achieved using: ``` const subscription = emitter.addListener('event', () => { subscription.remove(); // ... }); ``` Changelog: [General][Removed] - Removed `once()` and `removeCurrentListener()` fom `DeviceEventEmitter` and `NativeEventEmitter`. Reviewed By: cpojer Differential Revision: D22196474 fbshipit-source-id: 06ced186fd812e91d5c57f6580e647c100505807 --- Libraries/vendor/emitter/_EventEmitter.js | 53 ----------------------- 1 file changed, 53 deletions(-) diff --git a/Libraries/vendor/emitter/_EventEmitter.js b/Libraries/vendor/emitter/_EventEmitter.js index b55f487ebe..8ec88d27d0 100644 --- a/Libraries/vendor/emitter/_EventEmitter.js +++ b/Libraries/vendor/emitter/_EventEmitter.js @@ -33,7 +33,6 @@ const sparseFilterPredicate = () => true; */ class EventEmitter { _subscriber: EventSubscriptionVendor; - _currentSubscription: ?EmitterSubscription; /** * @constructor @@ -70,27 +69,6 @@ class EventEmitter { ): any); } - /** - * Similar to addListener, except that the listener is removed after it is - * invoked once. - * - * @param {string} eventType - Name of the event to listen to - * @param {function} listener - Function to invoke only once when the - * specified event is emitted - * @param {*} context - Optional context object to use when invoking the - * listener - */ - once( - eventType: string, - listener: Function, - context: ?Object, - ): EmitterSubscription { - return this.addListener(eventType, (...args) => { - this.removeCurrentListener(); - listener.apply(context, args); - }); - } - /** * Removes all of the registered listeners, including those registered as * listener maps. @@ -102,35 +80,6 @@ class EventEmitter { this._subscriber.removeAllSubscriptions(eventType); } - /** - * Provides an API that can be called during an eventing cycle to remove the - * last listener that was invoked. This allows a developer to provide an event - * object that can remove the listener (or listener map) during the - * invocation. - * - * If it is called when not inside of an emitting cycle it will throw. - * - * @throws {Error} When called not during an eventing cycle - * - * @example - * var subscription = emitter.addListenerMap({ - * someEvent: function(data, event) { - * console.log(data); - * emitter.removeCurrentListener(); - * } - * }); - * - * emitter.emit('someEvent', 'abc'); // logs 'abc' - * emitter.emit('someEvent', 'def'); // does not log anything - */ - removeCurrentListener() { - invariant( - !!this._currentSubscription, - 'Not in an emitting cycle; there is no current subscription', - ); - this.removeSubscription(this._currentSubscription); - } - /** * Removes a specific subscription. Called by the `remove()` method of the * subscription itself to ensure any necessary cleanup is performed. @@ -185,14 +134,12 @@ class EventEmitter { // The subscription may have been removed during this event loop. if (subscription && subscription.listener) { - this._currentSubscription = subscription; subscription.listener.apply( subscription.context, Array.prototype.slice.call(arguments, 1), ); } } - this._currentSubscription = null; } } From 4a1820dbdf999548772701da9f33d2b9a9304389 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Sat, 27 Jun 2020 02:15:58 -0700 Subject: [PATCH 16/49] EventEmitter: Import `{Native,RCTDevice}EventEmitter` Summary: Upgrades `require` expressions of `NativeEventEmitter` and `RCTDeviceEventEmitter` to `import`. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D22203919 fbshipit-source-id: 4fdf01b66ba0501d0e0714931923531c97d29be2 --- IntegrationTests/AccessibilityManagerTest.js | 13 ++++++------- Libraries/AppState/AppState.js | 8 +++----- Libraries/BugReporting/BugReporting.js | 3 +-- .../AccessibilityInfo.android.js | 5 ++--- .../AccessibilityInfo/AccessibilityInfo.ios.js | 4 +--- Libraries/Components/AppleTV/TVEventHandler.js | 5 ++--- Libraries/Components/Keyboard/Keyboard.js | 10 ++++------ .../Keyboard/__tests__/Keyboard-test.js | 10 +++++----- Libraries/Components/StatusBar/StatusBarIOS.js | 3 +-- Libraries/EventEmitter/NativeEventEmitter.js | 8 +++----- .../EventEmitter/RCTNativeAppEventEmitter.js | 2 +- .../__mocks__/NativeEventEmitter.js | 3 +-- Libraries/Linking/Linking.js | 10 ++++------ Libraries/Network/RCTNetworking.android.js | 6 ++---- Libraries/Network/RCTNetworking.ios.js | 6 ++---- .../PushNotificationIOS/PushNotificationIOS.js | 4 ++-- Libraries/ReactNative/AppContainer.js | 11 +++++------ Libraries/Settings/Settings.ios.js | 6 ++---- Libraries/WebSocket/WebSocket.js | 18 ++++++++---------- Libraries/WebSocket/WebSocketInterceptor.js | 6 ++---- 20 files changed, 57 insertions(+), 84 deletions(-) diff --git a/IntegrationTests/AccessibilityManagerTest.js b/IntegrationTests/AccessibilityManagerTest.js index dd155cec22..7501170f3a 100644 --- a/IntegrationTests/AccessibilityManagerTest.js +++ b/IntegrationTests/AccessibilityManagerTest.js @@ -10,13 +10,12 @@ 'use strict'; -const React = require('react'); -const ReactNative = require('react-native'); -const {View} = ReactNative; -const RCTDeviceEventEmitter = require('react-native/Libraries/EventEmitter/RCTDeviceEventEmitter'); -const {TestModule} = ReactNative.NativeModules; -import NativeAccessibilityManager from 'react-native/Libraries/Components/AccessibilityInfo/NativeAccessibilityManager'; import invariant from 'invariant'; +import NativeAccessibilityManager from 'react-native/Libraries/Components/AccessibilityInfo/NativeAccessibilityManager'; +import {DeviceEventEmitter, NativeModules, View} from 'react-native'; +import * as React from 'react'; + +const {TestModule} = NativeModules; class AccessibilityManagerTest extends React.Component<{...}> { componentDidMount() { @@ -39,7 +38,7 @@ class AccessibilityManagerTest extends React.Component<{...}> { accessibilityExtraExtraLarge: 11.0, accessibilityExtraExtraExtraLarge: 12.0, }); - RCTDeviceEventEmitter.addListener('didUpdateDimensions', update => { + DeviceEventEmitter.addListener('didUpdateDimensions', update => { TestModule.markTestPassed(update.window.fontScale === 4.0); }); } diff --git a/Libraries/AppState/AppState.js b/Libraries/AppState/AppState.js index bdd799bf4e..cc8a62a381 100644 --- a/Libraries/AppState/AppState.js +++ b/Libraries/AppState/AppState.js @@ -10,13 +10,11 @@ 'use strict'; -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); - -const invariant = require('invariant'); -const logError = require('../Utilities/logError'); - +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; +import logError from '../Utilities/logError'; import EventEmitter from '../vendor/emitter/EventEmitter'; import NativeAppState from './NativeAppState'; +import invariant from 'invariant'; /** * `AppState` can tell you if the app is in the foreground or background, diff --git a/Libraries/BugReporting/BugReporting.js b/Libraries/BugReporting/BugReporting.js index e6ccae6cc3..d2f3c4a442 100644 --- a/Libraries/BugReporting/BugReporting.js +++ b/Libraries/BugReporting/BugReporting.js @@ -10,8 +10,7 @@ 'use strict'; -const RCTDeviceEventEmitter = require('../EventEmitter/RCTDeviceEventEmitter'); - +import RCTDeviceEventEmitter from '../EventEmitter/RCTDeviceEventEmitter'; import NativeRedBox from '../NativeModules/specs/NativeRedBox'; import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import NativeBugReporting from './NativeBugReporting'; diff --git a/Libraries/Components/AccessibilityInfo/AccessibilityInfo.android.js b/Libraries/Components/AccessibilityInfo/AccessibilityInfo.android.js index 9a99eb6fb6..3c77eb3c9f 100644 --- a/Libraries/Components/AccessibilityInfo/AccessibilityInfo.android.js +++ b/Libraries/Components/AccessibilityInfo/AccessibilityInfo.android.js @@ -10,9 +10,8 @@ 'use strict'; -const RCTDeviceEventEmitter = require('../../EventEmitter/RCTDeviceEventEmitter'); -const UIManager = require('../../ReactNative/UIManager'); - +import RCTDeviceEventEmitter from '../../EventEmitter/RCTDeviceEventEmitter'; +import UIManager from '../../ReactNative/UIManager'; import NativeAccessibilityInfo from './NativeAccessibilityInfo'; const REDUCE_MOTION_EVENT = 'reduceMotionDidChange'; diff --git a/Libraries/Components/AccessibilityInfo/AccessibilityInfo.ios.js b/Libraries/Components/AccessibilityInfo/AccessibilityInfo.ios.js index 9e721a776b..6df67ffaeb 100644 --- a/Libraries/Components/AccessibilityInfo/AccessibilityInfo.ios.js +++ b/Libraries/Components/AccessibilityInfo/AccessibilityInfo.ios.js @@ -10,9 +10,7 @@ 'use strict'; -const Promise = require('../../Promise'); -const RCTDeviceEventEmitter = require('../../EventEmitter/RCTDeviceEventEmitter'); - +import RCTDeviceEventEmitter from '../../EventEmitter/RCTDeviceEventEmitter'; import NativeAccessibilityManager from './NativeAccessibilityManager'; const CHANGE_EVENT_NAME = { diff --git a/Libraries/Components/AppleTV/TVEventHandler.js b/Libraries/Components/AppleTV/TVEventHandler.js index a7f12b5306..d3f475a1fd 100644 --- a/Libraries/Components/AppleTV/TVEventHandler.js +++ b/Libraries/Components/AppleTV/TVEventHandler.js @@ -10,9 +10,8 @@ 'use strict'; -const NativeEventEmitter = require('../../EventEmitter/NativeEventEmitter'); -const Platform = require('../../Utilities/Platform'); - +import NativeEventEmitter from '../../EventEmitter/NativeEventEmitter'; +import Platform from '../../Utilities/Platform'; import {type EventSubscription} from '../../vendor/emitter/EventEmitter'; import NativeTVNavigationEventEmitter from './NativeTVNavigationEventEmitter'; diff --git a/Libraries/Components/Keyboard/Keyboard.js b/Libraries/Components/Keyboard/Keyboard.js index c10fe53dad..a9e0a29771 100644 --- a/Libraries/Components/Keyboard/Keyboard.js +++ b/Libraries/Components/Keyboard/Keyboard.js @@ -10,13 +10,11 @@ 'use strict'; -const LayoutAnimation = require('../../LayoutAnimation/LayoutAnimation'); -const NativeEventEmitter = require('../../EventEmitter/NativeEventEmitter'); - -const dismissKeyboard = require('../../Utilities/dismissKeyboard'); -const invariant = require('invariant'); - +import NativeEventEmitter from '../../EventEmitter/NativeEventEmitter'; +import LayoutAnimation from '../../LayoutAnimation/LayoutAnimation'; +import dismissKeyboard from '../../Utilities/dismissKeyboard'; import NativeKeyboardObserver from './NativeKeyboardObserver'; +import invariant from 'invariant'; const KeyboardEventEmitter: NativeEventEmitter = new NativeEventEmitter( NativeKeyboardObserver, ); diff --git a/Libraries/Components/Keyboard/__tests__/Keyboard-test.js b/Libraries/Components/Keyboard/__tests__/Keyboard-test.js index 181eb14b77..639cdd0db5 100644 --- a/Libraries/Components/Keyboard/__tests__/Keyboard-test.js +++ b/Libraries/Components/Keyboard/__tests__/Keyboard-test.js @@ -11,12 +11,12 @@ 'use strict'; -const Keyboard = require('../Keyboard'); -const dismissKeyboard = require('../../../Utilities/dismissKeyboard'); -const LayoutAnimation = require('../../../LayoutAnimation/LayoutAnimation'); - -const NativeEventEmitter = require('../../../EventEmitter/NativeEventEmitter'); const NativeModules = require('../../../BatchedBridge/NativeModules'); +const LayoutAnimation = require('../../../LayoutAnimation/LayoutAnimation'); +const dismissKeyboard = require('../../../Utilities/dismissKeyboard'); +const Keyboard = require('../Keyboard'); + +import NativeEventEmitter from '../../../EventEmitter/NativeEventEmitter'; jest.mock('../../../LayoutAnimation/LayoutAnimation'); diff --git a/Libraries/Components/StatusBar/StatusBarIOS.js b/Libraries/Components/StatusBar/StatusBarIOS.js index 004f1fdee3..ccc561103f 100644 --- a/Libraries/Components/StatusBar/StatusBarIOS.js +++ b/Libraries/Components/StatusBar/StatusBarIOS.js @@ -10,8 +10,7 @@ 'use strict'; -const NativeEventEmitter = require('../../EventEmitter/NativeEventEmitter'); - +import NativeEventEmitter from '../../EventEmitter/NativeEventEmitter'; import NativeStatusBarManagerIOS from './NativeStatusBarManagerIOS'; /** diff --git a/Libraries/EventEmitter/NativeEventEmitter.js b/Libraries/EventEmitter/NativeEventEmitter.js index 8ff7ab7724..606821725e 100644 --- a/Libraries/EventEmitter/NativeEventEmitter.js +++ b/Libraries/EventEmitter/NativeEventEmitter.js @@ -10,13 +10,11 @@ 'use strict'; -const Platform = require('../Utilities/Platform'); -const RCTDeviceEventEmitter = require('./RCTDeviceEventEmitter'); - -const invariant = require('invariant'); - +import Platform from '../Utilities/Platform'; import EventEmitter from '../vendor/emitter/EventEmitter'; import {type EventSubscription} from '../vendor/emitter/EventEmitter'; +import RCTDeviceEventEmitter from './RCTDeviceEventEmitter'; +import invariant from 'invariant'; type NativeModule = { +addListener: (eventType: string) => void, diff --git a/Libraries/EventEmitter/RCTNativeAppEventEmitter.js b/Libraries/EventEmitter/RCTNativeAppEventEmitter.js index c0cbef506e..0e3c4faf51 100644 --- a/Libraries/EventEmitter/RCTNativeAppEventEmitter.js +++ b/Libraries/EventEmitter/RCTNativeAppEventEmitter.js @@ -10,7 +10,7 @@ 'use strict'; -const RCTDeviceEventEmitter = require('./RCTDeviceEventEmitter'); +import RCTDeviceEventEmitter from './RCTDeviceEventEmitter'; /** * Deprecated - subclass NativeEventEmitter to create granular event modules instead of diff --git a/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js b/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js index c507d72afb..35f46742ab 100644 --- a/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js +++ b/Libraries/EventEmitter/__mocks__/NativeEventEmitter.js @@ -10,9 +10,8 @@ 'use strict'; -const RCTDeviceEventEmitter = require('../RCTDeviceEventEmitter'); - import EventEmitter from '../../vendor/emitter/EventEmitter'; +import RCTDeviceEventEmitter from '../RCTDeviceEventEmitter'; /** * Mock the NativeEventEmitter as a normal JS EventEmitter. diff --git a/Libraries/Linking/Linking.js b/Libraries/Linking/Linking.js index dd6fc76f15..7a0ab925a6 100644 --- a/Libraries/Linking/Linking.js +++ b/Libraries/Linking/Linking.js @@ -10,13 +10,11 @@ 'use strict'; -const InteractionManager = require('../Interaction/InteractionManager'); -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); -const Platform = require('../Utilities/Platform'); - -const invariant = require('invariant'); - +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; +import InteractionManager from '../Interaction/InteractionManager'; +import Platform from '../Utilities/Platform'; import NativeLinking from './NativeLinking'; +import invariant from 'invariant'; /** * `Linking` gives you a general interface to interact with both incoming diff --git a/Libraries/Network/RCTNetworking.android.js b/Libraries/Network/RCTNetworking.android.js index b1577a078f..12864c9c9a 100644 --- a/Libraries/Network/RCTNetworking.android.js +++ b/Libraries/Network/RCTNetworking.android.js @@ -12,11 +12,9 @@ // Do not require the native RCTNetworking module directly! Use this wrapper module instead. // It will add the necessary requestId, so that you don't have to generate it yourself. -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); - -const convertRequestBody = require('./convertRequestBody'); - +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; import NativeNetworkingAndroid from './NativeNetworkingAndroid'; +import convertRequestBody from './convertRequestBody'; import type {RequestBody} from './convertRequestBody'; type Header = [string, string]; diff --git a/Libraries/Network/RCTNetworking.ios.js b/Libraries/Network/RCTNetworking.ios.js index 73a33ab1d9..ec3706bf0e 100644 --- a/Libraries/Network/RCTNetworking.ios.js +++ b/Libraries/Network/RCTNetworking.ios.js @@ -10,12 +10,10 @@ 'use strict'; -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); - -const convertRequestBody = require('./convertRequestBody'); - +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; import NativeNetworkingIOS from './NativeNetworkingIOS'; import type {NativeResponseType} from './XMLHttpRequest'; +import convertRequestBody from './convertRequestBody'; import type {RequestBody} from './convertRequestBody'; class RCTNetworking extends NativeEventEmitter { diff --git a/Libraries/PushNotificationIOS/PushNotificationIOS.js b/Libraries/PushNotificationIOS/PushNotificationIOS.js index 73c184e749..99d2d92c97 100644 --- a/Libraries/PushNotificationIOS/PushNotificationIOS.js +++ b/Libraries/PushNotificationIOS/PushNotificationIOS.js @@ -10,9 +10,9 @@ 'use strict'; -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; import NativePushNotificationManagerIOS from './NativePushNotificationManagerIOS'; -const invariant = require('invariant'); +import invariant from 'invariant'; const PushNotificationEmitter = new NativeEventEmitter( NativePushNotificationManagerIOS, diff --git a/Libraries/ReactNative/AppContainer.js b/Libraries/ReactNative/AppContainer.js index 66eafadc79..68dd3169dd 100644 --- a/Libraries/ReactNative/AppContainer.js +++ b/Libraries/ReactNative/AppContainer.js @@ -10,14 +10,13 @@ 'use strict'; -const PropTypes = require('prop-types'); -const RCTDeviceEventEmitter = require('../EventEmitter/RCTDeviceEventEmitter'); -const React = require('react'); -const StyleSheet = require('../StyleSheet/StyleSheet'); -const View = require('../Components/View/View'); - +import View from '../Components/View/View'; +import RCTDeviceEventEmitter from '../EventEmitter/RCTDeviceEventEmitter'; +import StyleSheet from '../StyleSheet/StyleSheet'; import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import {RootTagContext, createRootTag} from './RootTag'; +import PropTypes from 'prop-types'; +import * as React from 'react'; type Context = {rootTag: number, ...}; diff --git a/Libraries/Settings/Settings.ios.js b/Libraries/Settings/Settings.ios.js index cba3973403..cac0b3babd 100644 --- a/Libraries/Settings/Settings.ios.js +++ b/Libraries/Settings/Settings.ios.js @@ -10,11 +10,9 @@ 'use strict'; -const RCTDeviceEventEmitter = require('../EventEmitter/RCTDeviceEventEmitter'); - -const invariant = require('invariant'); - +import RCTDeviceEventEmitter from '../EventEmitter/RCTDeviceEventEmitter'; import NativeSettingsManager from './NativeSettingsManager'; +import invariant from 'invariant'; const subscriptions: Array<{ keys: Array, diff --git a/Libraries/WebSocket/WebSocket.js b/Libraries/WebSocket/WebSocket.js index 5023a0b07f..65cfd09c52 100644 --- a/Libraries/WebSocket/WebSocket.js +++ b/Libraries/WebSocket/WebSocket.js @@ -10,18 +10,16 @@ 'use strict'; -const Blob = require('../Blob/Blob'); -const BlobManager = require('../Blob/BlobManager'); -const EventTarget = require('event-target-shim'); -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); -const WebSocketEvent = require('./WebSocketEvent'); - -const base64 = require('base64-js'); -const binaryToBase64 = require('../Utilities/binaryToBase64'); -const invariant = require('invariant'); - +import Blob from '../Blob/Blob'; +import BlobManager from '../Blob/BlobManager'; +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; +import binaryToBase64 from '../Utilities/binaryToBase64'; import {type EventSubscription} from '../vendor/emitter/EventEmitter'; import NativeWebSocketModule from './NativeWebSocketModule'; +import WebSocketEvent from './WebSocketEvent'; +import base64 from 'base64-js'; +import EventTarget from 'event-target-shim'; +import invariant from 'invariant'; type ArrayBufferView = | Int8Array diff --git a/Libraries/WebSocket/WebSocketInterceptor.js b/Libraries/WebSocket/WebSocketInterceptor.js index 6c371c1028..7882c46f3f 100644 --- a/Libraries/WebSocket/WebSocketInterceptor.js +++ b/Libraries/WebSocket/WebSocketInterceptor.js @@ -9,11 +9,9 @@ 'use strict'; -const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter'); - +import NativeEventEmitter from '../EventEmitter/NativeEventEmitter'; import NativeWebSocketModule from './NativeWebSocketModule'; - -const base64 = require('base64-js'); +import base64 from 'base64-js'; const originalRCTWebSocketConnect = NativeWebSocketModule.connect; const originalRCTWebSocketSend = NativeWebSocketModule.send; From 160f6ca54a331686513168a6517fcebcead467c1 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Sun, 28 Jun 2020 03:38:49 -0700 Subject: [PATCH 17/49] Make TextInput accessible element Summary: Changelog: [Internal] RCTTextInputComponentView didn't override `accessibilityElement`. This diff fixes it. Reviewed By: shergin Differential Revision: D22233182 fbshipit-source-id: 38fc2112aeabd514cd201920616e7d2b32e11d9b --- .../ComponentViews/TextInput/RCTTextInputComponentView.mm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm index eb312a44d3..a43ad335a9 100644 --- a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm @@ -89,6 +89,11 @@ using namespace facebook::react; } } +- (NSObject *)accessibilityElement +{ + return _backedTextInputView; +} + #pragma mark - RCTComponentViewProtocol + (ComponentDescriptorProvider)componentDescriptorProvider From 66057efc6d2bafac55619a5726c9bd96f809c0aa Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Sun, 28 Jun 2020 03:38:49 -0700 Subject: [PATCH 18/49] Organise methods in RCTTextInputComponentView into groups Summary: Changelog: [Internal] The methods were out of order, this diff reorders them. Reviewed By: shergin Differential Revision: D22233255 fbshipit-source-id: ea66bc701c4d021ec5721e191bc0d3413b3d36ae --- .../TextInput/RCTTextInputComponentView.mm | 126 +++++++++--------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm index a43ad335a9..3bf312aa25 100644 --- a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm @@ -58,6 +58,8 @@ using namespace facebook::react; BOOL _didMoveToWindow; } +#pragma mark - UIView overrides + - (instancetype)initWithFrame:(CGRect)frame { if (self = [super initWithFrame:frame]) { @@ -89,6 +91,8 @@ using namespace facebook::react; } } +#pragma mark - RCTViewComponentView overrides + - (NSObject *)accessibilityElement { return _backedTextInputView; @@ -235,20 +239,6 @@ using namespace facebook::react; RCTUIEdgeInsetsFromEdgeInsets(layoutMetrics.contentInsets - layoutMetrics.borderWidth); } -- (void)_setAttributedString:(NSAttributedString *)attributedString -{ - UITextRange *selectedRange = [_backedTextInputView selectedTextRange]; - _backedTextInputView.attributedText = attributedString; - if (_lastStringStateWasUpdatedWith.length == attributedString.length) { - // Calling `[_backedTextInputView setAttributedText]` moves caret - // to the end of text input field. This cancels any selection as well - // as position in the text input field. In case the length of string - // doesn't change, selection and caret position is maintained. - [_backedTextInputView setSelectedTextRange:selectedRange notifyDelegate:NO]; - } - _lastStringStateWasUpdatedWith = attributedString; -} - - (void)prepareForRecycle { [super prepareForRecycle]; @@ -261,19 +251,6 @@ using namespace facebook::react; _didMoveToWindow = NO; } -#pragma mark - RCTComponentViewProtocol - -- (void)_setMultiline:(BOOL)multiline -{ - [_backedTextInputView removeFromSuperview]; - UIView *backedTextInputView = - multiline ? [[RCTUITextView alloc] init] : [[RCTUITextField alloc] init]; - backedTextInputView.frame = _backedTextInputView.frame; - RCTCopyBackedTextInput(_backedTextInputView, backedTextInputView); - _backedTextInputView = backedTextInputView; - [self addSubview:_backedTextInputView]; -} - #pragma mark - RCTBackedTextInputDelegate - (BOOL)textInputShouldBeginEditing @@ -398,41 +375,6 @@ using namespace facebook::react; } } -#pragma mark - Other - -- (TextInputMetrics)_textInputMetrics -{ - TextInputMetrics metrics; - metrics.text = RCTStringFromNSString(_backedTextInputView.attributedText.string); - metrics.selectionRange = [self _selectionRange]; - metrics.eventCount = _mostRecentEventCount; - return metrics; -} - -- (void)_updateState -{ - if (!_state) { - return; - } - NSAttributedString *attributedString = _backedTextInputView.attributedText; - auto data = _state->getData(); - _lastStringStateWasUpdatedWith = attributedString; - data.attributedStringBox = RCTAttributedStringBoxFromNSAttributedString(attributedString); - _mostRecentEventCount += _comingFromJS ? 0 : 1; - data.mostRecentEventCount = _mostRecentEventCount; - _state->updateState(std::move(data)); -} - -- (AttributedString::Range)_selectionRange -{ - UITextRange *selectedTextRange = _backedTextInputView.selectedTextRange; - NSInteger start = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument - toPosition:selectedTextRange.start]; - NSInteger end = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument - toPosition:selectedTextRange.end]; - return AttributedString::Range{(int)start, (int)(end - start)}; -} - #pragma mark - Native Commands - (void)handleCommand:(const NSString *)commandName args:(const NSArray *)args @@ -480,6 +422,66 @@ using namespace facebook::react; _comingFromJS = NO; } +#pragma mark - Other + +- (TextInputMetrics)_textInputMetrics +{ + TextInputMetrics metrics; + metrics.text = RCTStringFromNSString(_backedTextInputView.attributedText.string); + metrics.selectionRange = [self _selectionRange]; + metrics.eventCount = _mostRecentEventCount; + return metrics; +} + +- (void)_updateState +{ + if (!_state) { + return; + } + NSAttributedString *attributedString = _backedTextInputView.attributedText; + auto data = _state->getData(); + _lastStringStateWasUpdatedWith = attributedString; + data.attributedStringBox = RCTAttributedStringBoxFromNSAttributedString(attributedString); + _mostRecentEventCount += _comingFromJS ? 0 : 1; + data.mostRecentEventCount = _mostRecentEventCount; + _state->updateState(std::move(data)); +} + +- (AttributedString::Range)_selectionRange +{ + UITextRange *selectedTextRange = _backedTextInputView.selectedTextRange; + NSInteger start = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument + toPosition:selectedTextRange.start]; + NSInteger end = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument + toPosition:selectedTextRange.end]; + return AttributedString::Range{(int)start, (int)(end - start)}; +} + +- (void)_setAttributedString:(NSAttributedString *)attributedString +{ + UITextRange *selectedRange = [_backedTextInputView selectedTextRange]; + _backedTextInputView.attributedText = attributedString; + if (_lastStringStateWasUpdatedWith.length == attributedString.length) { + // Calling `[_backedTextInputView setAttributedText]` moves caret + // to the end of text input field. This cancels any selection as well + // as position in the text input field. In case the length of string + // doesn't change, selection and caret position is maintained. + [_backedTextInputView setSelectedTextRange:selectedRange notifyDelegate:NO]; + } + _lastStringStateWasUpdatedWith = attributedString; +} + +- (void)_setMultiline:(BOOL)multiline +{ + [_backedTextInputView removeFromSuperview]; + UIView *backedTextInputView = + multiline ? [[RCTUITextView alloc] init] : [[RCTUITextField alloc] init]; + backedTextInputView.frame = _backedTextInputView.frame; + RCTCopyBackedTextInput(_backedTextInputView, backedTextInputView); + _backedTextInputView = backedTextInputView; + [self addSubview:_backedTextInputView]; +} + @end Class RCTTextInputCls(void) From a50f736bb6ade9ea9caae45e41ca4b92f6707b17 Mon Sep 17 00:00:00 2001 From: Marco Scabbiolo Date: Sun, 28 Jun 2020 23:46:20 -0700 Subject: [PATCH 19/49] Add DevSettings to jest preset (#29223) Summary: Provide a mocked `DevSettings` implementation for jest. ## Changelog [General] [Added] - Add mock for `DevSettings` to jest preset Pull Request resolved: https://github.com/facebook/react-native/pull/29223 Test Plan: None Reviewed By: cpojer Differential Revision: D22279962 Pulled By: TheSavior fbshipit-source-id: 667cecb0a558a4267564702ee9d30380756bdd92 --- jest/setup.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jest/setup.js b/jest/setup.js index 5f85d8602f..7e9eb68048 100644 --- a/jest/setup.js +++ b/jest/setup.js @@ -211,6 +211,10 @@ jest }; }, }, + DevSettings: { + addMenuItem: jest.fn(), + reload: jest.fn(), + }, ImageLoader: { getSize: jest.fn(url => Promise.resolve({width: 320, height: 240})), prefetchImage: jest.fn(), From f8c68bf358313a2a23fd8f31e5f363b53f721010 Mon Sep 17 00:00:00 2001 From: "Andrew Coates (REDMOND)" Date: Mon, 29 Jun 2020 11:58:03 -0700 Subject: [PATCH 20/49] Make platformColors test page render with out of tree platforms (#29120) Summary: PlatformColorExample RNTester page crashes when running on platforms other than iOS/Android. Changed some very minor logic to take into account that platforms other than iOS/Android exist. ## Changelog [Internal] [Fixed] - PlatformColorExample RNTester page crashes when running on platforms other than iOS/Android. Pull Request resolved: https://github.com/facebook/react-native/pull/29120 Reviewed By: sahrens Differential Revision: D22109188 Pulled By: TheSavior fbshipit-source-id: 853bea0ac4ea5c67f35d89d1a2bf55f5c89823b4 --- .../PlatformColor/PlatformColorExample.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/RNTester/js/examples/PlatformColor/PlatformColorExample.js b/RNTester/js/examples/PlatformColor/PlatformColorExample.js index 5cb831c062..77c3ebbe47 100644 --- a/RNTester/js/examples/PlatformColor/PlatformColorExample.js +++ b/RNTester/js/examples/PlatformColor/PlatformColorExample.js @@ -199,7 +199,10 @@ function FallbackColorsExample() { color: PlatformColor('bogus', '@color/catalyst_redbox_background'), }; } else { - throw 'Unexpected Platform.OS: ' + Platform.OS; + color = { + label: 'Unexpected Platform.OS: ' + Platform.OS, + color: 'red', + }; } return ( @@ -261,9 +264,11 @@ function VariantColorsExample() { - {Platform.OS === 'ios' - ? "DynamicColorIOS({light: 'red', dark: 'blue'})" - : "PlatformColor('?attr/colorAccent')"} + {Platform.select({ + ios: "DynamicColorIOS({light: 'red', dark: 'blue'})", + android: "PlatformColor('?attr/colorAccent')", + default: 'Unexpected Platform.OS: ' + Platform.OS, + })} From 72285d808dfce748287a19e2620d58517a5f76e7 Mon Sep 17 00:00:00 2001 From: Chris Dadabo <2096864+CMDadabo@users.noreply.github.com> Date: Mon, 29 Jun 2020 12:55:20 -0700 Subject: [PATCH 21/49] Add accessibilityHint to TouchableNativeFeedback (#29154) Summary: The docs suggest that TouchableNativeFeedback [inherits `TouchableWithoutFeedback` props](https://reactnative.dev/docs/touchablewithoutfeedback#props) but `accessibilityHint` was missing. ## Changelog [Android] [Added] - Add accessibilityHint to TouchableNativeFeedback Pull Request resolved: https://github.com/facebook/react-native/pull/29154 Test Plan: Not sure how this should be tested, but I'm happy to implement what others may suggest Reviewed By: kacieb Differential Revision: D22109459 Pulled By: TheSavior fbshipit-source-id: 573267a26414a97ba23a1a7995bff1608f9ba34f --- Libraries/Components/Touchable/TouchableNativeFeedback.js | 1 + 1 file changed, 1 insertion(+) diff --git a/Libraries/Components/Touchable/TouchableNativeFeedback.js b/Libraries/Components/Touchable/TouchableNativeFeedback.js index 5dc03df877..dda6c4bf37 100644 --- a/Libraries/Components/Touchable/TouchableNativeFeedback.js +++ b/Libraries/Components/Touchable/TouchableNativeFeedback.js @@ -271,6 +271,7 @@ class TouchableNativeFeedback extends React.Component { this.props.useForeground === true, ), accessible: this.props.accessible !== false, + accessibilityHint: this.props.accessibilityHint, accessibilityLabel: this.props.accessibilityLabel, accessibilityRole: this.props.accessibilityRole, accessibilityState: this.props.accessibilityState, From 6d447944475ba98ee790daa520cbd3151cdc1df2 Mon Sep 17 00:00:00 2001 From: Eli White Date: Mon, 29 Jun 2020 13:11:24 -0700 Subject: [PATCH 22/49] Bump react-native-codegen: 0.0.3 Summary: Need to publish a new version now that we want to not publish Flow code Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: kacieb Differential Revision: D22265050 fbshipit-source-id: a9a0d03b1e2c1ec72e642b0af070ba48426825f8 --- packages/react-native-codegen/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-native-codegen/package.json b/packages/react-native-codegen/package.json index 2bcba86fde..a68d170a74 100644 --- a/packages/react-native-codegen/package.json +++ b/packages/react-native-codegen/package.json @@ -1,6 +1,6 @@ { "name": "react-native-codegen", - "version": "0.0.2", + "version": "0.0.3", "description": "⚛️ Code generation tools for React Native", "homepage": "https://github.com/facebook/react-native/tree/master/packages/react-native-codegen", "repository": { From fb2b49d2982399f26b93ac02b2197c0b3813a4a9 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Mon, 29 Jun 2020 15:24:35 -0700 Subject: [PATCH 23/49] Remove integration between Paper and Fabric's UIManager Summary: Changelog: [Internal] In D8552360 (https://github.com/facebook/react-native/commit/48b9a6f887c09f78944053a6658106a01dc20fea) an experimental integration between old and new UIManager has been introduced. It isn't needed anymore. I did some measurements and it is the slowest part of `[RCTComponentViewDescriptor dequeueComponentViewWithComponentHandle]`. {F241943384} Reviewed By: shergin Differential Revision: D22274782 fbshipit-source-id: 799ba047f1c57f68f00b0b6fa7c58782874991bc --- .../Mounting/RCTComponentViewRegistry.mm | 62 ------------------- React/Fabric/RCTSurfacePresenter.h | 5 ++ React/Fabric/RCTSurfacePresenter.mm | 7 +++ React/Modules/RCTSurfacePresenterStub.h | 1 + React/Modules/RCTUIManager.m | 7 ++- 5 files changed, 19 insertions(+), 63 deletions(-) diff --git a/React/Fabric/Mounting/RCTComponentViewRegistry.mm b/React/Fabric/Mounting/RCTComponentViewRegistry.mm index b0dbe553b0..870f49352c 100644 --- a/React/Fabric/Mounting/RCTComponentViewRegistry.mm +++ b/React/Fabric/Mounting/RCTComponentViewRegistry.mm @@ -18,60 +18,6 @@ using namespace facebook::react; -#define LEGACY_UIMANAGER_INTEGRATION_ENABLED 1 - -#ifdef LEGACY_UIMANAGER_INTEGRATION_ENABLED - -#import -#import - -/** - * Warning: This is a total hack and temporary solution. - * Unless we have a pure Fabric-based implementation of UIManager commands - * delivery pipeline, we have to leverage existing infra. This code tricks - * legacy UIManager by registering all Fabric-managed views in it, - * hence existing command-delivery infra can reach "foreign" views using - * the old pipeline. - */ -@interface RCTUIManager () -- (NSMutableDictionary *)viewRegistry; -@end - -@interface RCTUIManager (Hack) - -+ (void)registerView:(UIView *)view; -+ (void)unregisterView:(UIView *)view; - -@end - -@implementation RCTUIManager (Hack) - -+ (void)registerView:(UIView *)view -{ - if (!view) { - return; - } - - RCTUIManager *uiManager = [[RCTBridge currentBridge] uiManager]; - view.reactTag = @(view.tag); - [uiManager.viewRegistry setObject:view forKey:@(view.tag)]; -} - -+ (void)unregisterView:(UIView *)view -{ - if (!view) { - return; - } - - RCTUIManager *uiManager = [[RCTBridge currentBridge] uiManager]; - view.reactTag = nil; - [uiManager.viewRegistry removeObjectForKey:@(view.tag)]; -} - -@end - -#endif - const NSInteger RCTComponentViewRegistryRecyclePoolMaxSize = 1024; @implementation RCTComponentViewRegistry { @@ -133,10 +79,6 @@ const NSInteger RCTComponentViewRegistryRecyclePoolMaxSize = 1024; _registry.insert({tag, componentViewDescriptor}); -#ifdef LEGACY_UIMANAGER_INTEGRATION_ENABLED - [RCTUIManager registerView:componentViewDescriptor.view]; -#endif - return componentViewDescriptor; } @@ -149,10 +91,6 @@ const NSInteger RCTComponentViewRegistryRecyclePoolMaxSize = 1024; RCTAssert( _registry.find(tag) != _registry.end(), @"RCTComponentViewRegistry: Attempt to enqueue unregistered component."); -#ifdef LEGACY_UIMANAGER_INTEGRATION_ENABLED - [RCTUIManager unregisterView:componentViewDescriptor.view]; -#endif - _registry.erase(tag); componentViewDescriptor.view.tag = 0; [self _enqueueComponentViewWithComponentHandle:componentHandle componentViewDescriptor:componentViewDescriptor]; diff --git a/React/Fabric/RCTSurfacePresenter.h b/React/Fabric/RCTSurfacePresenter.h index abacc2f747..6293ef6167 100644 --- a/React/Fabric/RCTSurfacePresenter.h +++ b/React/Fabric/RCTSurfacePresenter.h @@ -75,6 +75,11 @@ NS_ASSUME_NONNULL_BEGIN - (void)removeObserver:(id)observer; +/* + * Please do not use this, this will be deleted soon. + */ +- (nullable UIView *)findComponentViewWithTag_DO_NOT_USE_DEPRECATED:(NSInteger)tag; + @end NS_ASSUME_NONNULL_END diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index 86c6588d11..2d4e6593a6 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -189,6 +189,13 @@ static inline LayoutContext RCTGetLayoutContext() surfaceId:surface.rootTag]; } +- (UIView *)findComponentViewWithTag_DO_NOT_USE_DEPRECATED:(NSInteger)tag +{ + UIView *componentView = + [_mountingManager.componentViewRegistry findComponentViewWithTag:tag]; + return componentView; +} + - (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props { RCTScheduler *scheduler = [self _scheduler]; diff --git a/React/Modules/RCTSurfacePresenterStub.h b/React/Modules/RCTSurfacePresenterStub.h index 402453e1bb..32a2b80e3c 100644 --- a/React/Modules/RCTSurfacePresenterStub.h +++ b/React/Modules/RCTSurfacePresenterStub.h @@ -26,6 +26,7 @@ NS_ASSUME_NONNULL_BEGIN @protocol RCTSurfacePresenterStub +- (nullable UIView *)findComponentViewWithTag_DO_NOT_USE_DEPRECATED:(NSInteger)tag; - (BOOL)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag props:(NSDictionary *)props; - (void)addObserver:(id)observer; - (void)removeObserver:(id)observer; diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index f0f6ca916e..26573fe3f4 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -8,6 +8,7 @@ #import "RCTUIManager.h" #import +#import #import "RCTAssert.h" #import "RCTBridge+Private.h" @@ -354,7 +355,11 @@ static NSDictionary *deviceOrientationEventBody(UIDeviceOrientation orientation) - (UIView *)viewForReactTag:(NSNumber *)reactTag { RCTAssertMainQueue(); - return _viewRegistry[reactTag]; + UIView *view = [_bridge.surfacePresenter findComponentViewWithTag_DO_NOT_USE_DEPRECATED:reactTag.integerValue]; + if (!view) { + view = _viewRegistry[reactTag]; + } + return view; } - (RCTShadowView *)shadowViewForReactTag:(NSNumber *)reactTag From 7abcaafd6600535825aa8330af7290ba8acea245 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Mon, 29 Jun 2020 15:25:40 -0700 Subject: [PATCH 24/49] Fix crash when updating dialog props after the Activity has disappeared Summary: This diff avoids accessing window and activities object that has dissapear changelog: [Android][Fix] Fix crash when updating RN dialog props after the activity disappeared Reviewed By: JoshuaGross Differential Revision: D22264672 fbshipit-source-id: 89c9895c8c6b6fec383a0e160847e5059616e265 --- .../react/views/modal/ReactModalHostView.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.java b/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.java index d5312acdb1..e404e0d4e9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/modal/ReactModalHostView.java @@ -17,6 +17,7 @@ import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; import android.view.ViewStructure; +import android.view.Window; import android.view.WindowManager; import android.view.accessibility.AccessibilityEvent; import android.widget.FrameLayout; @@ -347,24 +348,26 @@ public class ReactModalHostView extends ViewGroup implements LifecycleEventListe Assertions.assertNotNull(mDialog, "mDialog must exist when we call updateProperties"); Activity currentActivity = getCurrentActivity(); - if (currentActivity != null) { - int activityWindowFlags = currentActivity.getWindow().getAttributes().flags; - if ((activityWindowFlags & WindowManager.LayoutParams.FLAG_FULLSCREEN) != 0) { - mDialog.getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); - } else { - mDialog.getWindow().clearFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); - } + + Window window = mDialog.getWindow(); + if (currentActivity == null || currentActivity.isFinishing() || !window.isActive()) { + // If the activity has disappeared, then we shouldn't update the window associated to the + // Dialog. + return; + } + int activityWindowFlags = currentActivity.getWindow().getAttributes().flags; + if ((activityWindowFlags & WindowManager.LayoutParams.FLAG_FULLSCREEN) != 0) { + window.addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); + } else { + window.clearFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); } if (mTransparent) { - mDialog.getWindow().clearFlags(WindowManager.LayoutParams.FLAG_DIM_BEHIND); + window.clearFlags(WindowManager.LayoutParams.FLAG_DIM_BEHIND); } else { - mDialog.getWindow().setDimAmount(0.5f); - mDialog - .getWindow() - .setFlags( - WindowManager.LayoutParams.FLAG_DIM_BEHIND, - WindowManager.LayoutParams.FLAG_DIM_BEHIND); + window.setDimAmount(0.5f); + window.setFlags( + WindowManager.LayoutParams.FLAG_DIM_BEHIND, WindowManager.LayoutParams.FLAG_DIM_BEHIND); } } From 0a115d9c3f1b1b314b523d4440437a3dce093b44 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 29 Jun 2020 15:42:41 -0700 Subject: [PATCH 25/49] Ship MountItem Remove/Delete Collation Summary: Ship MountItem Remove/Delete Collation. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22170891 fbshipit-source-id: a9a149783bdd69df54e429da6ed9870bfb405a36 --- .../react/fabric/FabricJSIModuleProvider.java | 4 - .../react/fabric/FabricUIManager.java | 18 ----- .../com/facebook/react/fabric/jni/Binding.cpp | 76 +++++-------------- .../com/facebook/react/fabric/jni/Binding.h | 1 - .../fabric/mounting/MountingManager.java | 24 +++--- .../mounting/mountitems/DeleteMountItem.java | 30 -------- .../mounting/mountitems/RemoveMountItem.java | 47 ------------ 7 files changed, 30 insertions(+), 170 deletions(-) delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DeleteMountItem.java delete mode 100644 ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveMountItem.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java index 8b5b7263fc..136da3a752 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricJSIModuleProvider.java @@ -21,7 +21,6 @@ import com.facebook.react.fabric.mounting.LayoutMetricsConversions; import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.mountitems.BatchMountItem; import com.facebook.react.fabric.mounting.mountitems.CreateMountItem; -import com.facebook.react.fabric.mounting.mountitems.DeleteMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchIntCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchStringCommandMountItem; @@ -29,7 +28,6 @@ import com.facebook.react.fabric.mounting.mountitems.InsertMountItem; import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.fabric.mounting.mountitems.PreAllocateViewMountItem; import com.facebook.react.fabric.mounting.mountitems.RemoveDeleteMultiMountItem; -import com.facebook.react.fabric.mounting.mountitems.RemoveMountItem; import com.facebook.react.fabric.mounting.mountitems.SendAccessibilityEvent; import com.facebook.react.fabric.mounting.mountitems.UpdateEventEmitterMountItem; import com.facebook.react.fabric.mounting.mountitems.UpdateLayoutMountItem; @@ -112,7 +110,6 @@ public class FabricJSIModuleProvider implements JSIModuleProvider { FabricEventEmitter.class.getClass(); BatchMountItem.class.getClass(); CreateMountItem.class.getClass(); - DeleteMountItem.class.getClass(); DispatchCommandMountItem.class.getClass(); DispatchIntCommandMountItem.class.getClass(); DispatchStringCommandMountItem.class.getClass(); @@ -120,7 +117,6 @@ public class FabricJSIModuleProvider implements JSIModuleProvider { MountItem.class.getClass(); PreAllocateViewMountItem.class.getClass(); RemoveDeleteMultiMountItem.class.getClass(); - RemoveMountItem.class.getClass(); SendAccessibilityEvent.class.getClass(); UpdateEventEmitterMountItem.class.getClass(); UpdateLayoutMountItem.class.getClass(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 4bfbd11179..515228bc60 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -56,7 +56,6 @@ import com.facebook.react.fabric.events.FabricEventEmitter; import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.mountitems.BatchMountItem; import com.facebook.react.fabric.mounting.mountitems.CreateMountItem; -import com.facebook.react.fabric.mounting.mountitems.DeleteMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchIntCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchStringCommandMountItem; @@ -64,7 +63,6 @@ import com.facebook.react.fabric.mounting.mountitems.InsertMountItem; import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.fabric.mounting.mountitems.PreAllocateViewMountItem; import com.facebook.react.fabric.mounting.mountitems.RemoveDeleteMultiMountItem; -import com.facebook.react.fabric.mounting.mountitems.RemoveMountItem; import com.facebook.react.fabric.mounting.mountitems.SendAccessibilityEvent; import com.facebook.react.fabric.mounting.mountitems.UpdateEventEmitterMountItem; import com.facebook.react.fabric.mounting.mountitems.UpdateLayoutMountItem; @@ -367,14 +365,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { isLayoutable); } - @DoNotStrip - @SuppressWarnings("unused") - @AnyThread - @ThreadConfined(ANY) - private MountItem removeMountItem(int reactTag, int parentReactTag, int index) { - return new RemoveMountItem(reactTag, parentReactTag, index); - } - @DoNotStrip @SuppressWarnings("unused") @AnyThread @@ -383,14 +373,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { return new InsertMountItem(reactTag, parentReactTag, index); } - @DoNotStrip - @SuppressWarnings("unused") - @AnyThread - @ThreadConfined(ANY) - private MountItem deleteMountItem(int reactTag) { - return new DeleteMountItem(reactTag); - } - @DoNotStrip @SuppressWarnings("unused") @AnyThread diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index 3bb5ad4e16..715cf1bea9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -280,8 +280,6 @@ void Binding::installFabricUIManager( // Keep reference to config object and cache some feature flags here reactNativeConfig_ = config; - shouldCollateRemovesAndDeletes_ = reactNativeConfig_->getBool( - "react_fabric:enable_removedelete_collation_android"); collapseDeleteCreateMountingInstructions_ = reactNativeConfig_->getBool( "react_fabric:enabled_collapse_delete_create_mounting_instructions"); @@ -491,31 +489,6 @@ local_ref createUpdateStateMountItem( (javaStateWrapper != nullptr ? javaStateWrapper.get() : nullptr)); } -local_ref createRemoveMountItem( - const jni::global_ref &javaUIManager, - const ShadowViewMutation &mutation) { - static auto removeInstruction = - jni::findClassStatic(Binding::UIManagerJavaDescriptor) - ->getMethod(jint, jint, jint)>( - "removeMountItem"); - - return removeInstruction( - javaUIManager, - mutation.oldChildShadowView.tag, - mutation.parentShadowView.tag, - mutation.index); -} - -local_ref createDeleteMountItem( - const jni::global_ref &javaUIManager, - const ShadowViewMutation &mutation) { - static auto deleteInstruction = - jni::findClassStatic(Binding::UIManagerJavaDescriptor) - ->getMethod(jint)>("deleteMountItem"); - - return deleteInstruction(javaUIManager, mutation.oldChildShadowView.tag); -} - local_ref createRemoveAndDeleteMultiMountItem( const jni::global_ref &javaUIManager, const std::vector &metadata) { @@ -685,8 +658,7 @@ void Binding::schedulerDidFinishTransaction( oldChildShadowView.layoutMetrics == EmptyLayoutMetrics; // Handle accumulated removals/deletions - if (shouldCollateRemovesAndDeletes_ && - mutation.type != ShadowViewMutation::Remove && + if (mutation.type != ShadowViewMutation::Remove && mutation.type != ShadowViewMutation::Delete) { if (toRemove.size() > 0) { mountItems[position++] = @@ -708,39 +680,29 @@ void Binding::schedulerDidFinishTransaction( } case ShadowViewMutation::Remove: { if (!isVirtual) { - if (shouldCollateRemovesAndDeletes_) { - toRemove.push_back( - RemoveDeleteMetadata{mutation.oldChildShadowView.tag, - mutation.parentShadowView.tag, - mutation.index, - true, - false}); - } else { - mountItems[position++] = - createRemoveMountItem(localJavaUIManager, mutation); - } + toRemove.push_back( + RemoveDeleteMetadata{mutation.oldChildShadowView.tag, + mutation.parentShadowView.tag, + mutation.index, + true, + false}); } break; } case ShadowViewMutation::Delete: { - if (shouldCollateRemovesAndDeletes_) { - // It is impossible to delete without removing node first - const auto &it = std::find_if( - std::begin(toRemove), - std::end(toRemove), - [&mutation](const auto &x) { - return x.tag == mutation.oldChildShadowView.tag; - }); + // It is impossible to delete without removing node first + const auto &it = std::find_if( + std::begin(toRemove), + std::end(toRemove), + [&mutation](const auto &x) { + return x.tag == mutation.oldChildShadowView.tag; + }); - if (it != std::end(toRemove)) { - it->shouldDelete = true; - } else { - toRemove.push_back(RemoveDeleteMetadata{ - mutation.oldChildShadowView.tag, -1, -1, false, true}); - } + if (it != std::end(toRemove)) { + it->shouldDelete = true; } else { - mountItems[position++] = - createDeleteMountItem(localJavaUIManager, mutation); + toRemove.push_back(RemoveDeleteMetadata{ + mutation.oldChildShadowView.tag, -1, -1, false, true}); } deletedViewTags.insert(mutation.oldChildShadowView.tag); @@ -840,7 +802,7 @@ void Binding::schedulerDidFinishTransaction( } // Handle remaining removals and deletions - if (shouldCollateRemovesAndDeletes_ && toRemove.size() > 0) { + if (toRemove.size() > 0) { mountItems[position++] = createRemoveAndDeleteMultiMountItem(localJavaUIManager, toRemove); toRemove.clear(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h index e42883d35c..3d1d439f5e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.h @@ -125,7 +125,6 @@ class Binding : public jni::HybridClass, float pointScaleFactor_ = 1; std::shared_ptr reactNativeConfig_{nullptr}; - bool shouldCollateRemovesAndDeletes_{false}; bool collapseDeleteCreateMountingInstructions_{false}; bool disablePreallocateViews_{false}; bool disableVirtualNodePreallocation_{false}; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index fa3ee13304..47d6582698 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -281,19 +281,17 @@ public class MountingManager { ViewGroupManager viewGroupManager = getViewGroupManager(viewState); // Verify that the view we're about to remove has the same tag we expect - if (tag != -1) { - View view = viewGroupManager.getChildAt(parentView, index); - if (view != null && view.getId() != tag) { - throw new IllegalStateException( - "Tried to delete view [" - + tag - + "] of parent [" - + parentTag - + "] at index " - + index - + ", but got view tag " - + view.getId()); - } + View view = viewGroupManager.getChildAt(parentView, index); + if (view != null && view.getId() != tag) { + throw new IllegalStateException( + "Tried to delete view [" + + tag + + "] of parent [" + + parentTag + + "] at index " + + index + + ", but got view tag " + + view.getId()); } try { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DeleteMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DeleteMountItem.java deleted file mode 100644 index 312dcc1ca4..0000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/DeleteMountItem.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.fabric.mounting.mountitems; - -import androidx.annotation.NonNull; -import com.facebook.react.fabric.mounting.MountingManager; - -public class DeleteMountItem implements MountItem { - - private int mReactTag; - - public DeleteMountItem(int reactTag) { - mReactTag = reactTag; - } - - @Override - public void execute(@NonNull MountingManager mountingManager) { - mountingManager.deleteView(mReactTag); - } - - @Override - public String toString() { - return "DeleteMountItem [" + mReactTag + "]"; - } -} diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveMountItem.java deleted file mode 100644 index 3b6fd92110..0000000000 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveMountItem.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.fabric.mounting.mountitems; - -import androidx.annotation.NonNull; -import com.facebook.react.fabric.mounting.MountingManager; - -public class RemoveMountItem implements MountItem { - - private int mReactTag; - private int mParentReactTag; - private int mIndex; - - public RemoveMountItem(int reactTag, int parentReactTag, int index) { - mReactTag = reactTag; - mParentReactTag = parentReactTag; - mIndex = index; - } - - @Override - public void execute(@NonNull MountingManager mountingManager) { - mountingManager.removeViewAt(-1, mParentReactTag, mIndex); - } - - public int getParentReactTag() { - return mParentReactTag; - } - - public int getIndex() { - return mIndex; - } - - @Override - public String toString() { - return "RemoveMountItem [" - + mReactTag - + "] - parentTag: " - + mParentReactTag - + " - index: " - + mIndex; - } -} From 0b63c9464872fed9ff3eb388e32850f2f8c69561 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 29 Jun 2020 15:42:41 -0700 Subject: [PATCH 26/49] Ship early ViewCommand dispatch everywhere Summary: Early ViewCommand dispatch: ship the experiment everywhere on Android. Since ViewCommands are totally divorced from the commit cycle currently, and since they are inherently unsafe, we can create a separate queue for them and retry them if they fail with a specific category of exceptions. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22173050 fbshipit-source-id: 494b7c6b5dfd2aec8ba77ae35d0d58d4d727b9b4 --- .../react/config/ReactFeatureFlags.java | 6 --- .../react/fabric/FabricUIManager.java | 38 ++----------------- .../react/uimanager/UIViewOperationQueue.java | 15 +------- 3 files changed, 5 insertions(+), 54 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index 3cda1544d1..ce0b3d4dba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -57,12 +57,6 @@ public class ReactFeatureFlags { */ public static boolean nullifyCatalystInstanceOnDestroy = false; - /** - * Temporary flag. See UIImplementation: if this flag is enabled, ViewCommands will be queued and - * executed before any other types of UI operations. - */ - public static boolean allowEarlyViewCommandExecution = false; - /** * This react flag enables a custom algorithm for the getChildVisibleRect() method in the classes * ReactViewGroup, ReactHorizontalScrollView and ReactScrollView. diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 515228bc60..e76a5746e0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -683,10 +683,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @UiThread @ThreadConfined(UI) private List getAndResetViewCommandMountItems() { - if (!ReactFeatureFlags.allowEarlyViewCommandExecution) { - return null; - } - synchronized (mViewCommandMountItemsLock) { List result = mViewCommandMountItems; if (result.isEmpty()) { @@ -870,29 +866,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { } } - // TODO: if early ViewCommand dispatch ships 100% as a feature, this can be removed. - // This try/catch catches Retryable errors that can only be thrown by ViewCommands, which - // won't be executed here in Early Dispatch mode. - try { - mountItem.execute(mMountingManager); - } catch (RetryableMountingLayerException e) { - // It's very common for commands to be executed on views that no longer exist - for - // example, a blur event on TextInput being fired because of a navigation event away - // from the current screen. If the exception is marked as Retryable, we log a soft - // exception but never crash in debug. - // It's not clear that logging this is even useful, because these events are very - // common, mundane, and there's not much we can do about them currently. - if (mountItem instanceof DispatchCommandMountItem) { - ReactSoftException.logSoftException( - TAG, - new ReactNoCrashSoftException( - "Caught exception executing retryable mounting layer instruction: " - + mountItem.toString(), - e)); - } else { - throw e; - } - } + mountItem.execute(mMountingManager); } mBatchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime; } @@ -1027,14 +1001,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @AnyThread @ThreadConfined(ANY) private void dispatchCommandMountItem(DispatchCommandMountItem command) { - if (ReactFeatureFlags.allowEarlyViewCommandExecution) { - synchronized (mViewCommandMountItemsLock) { - mViewCommandMountItems.add(command); - } - } else { - synchronized (mMountItemsLock) { - mMountItems.add(command); - } + synchronized (mViewCommandMountItemsLock) { + mViewCommandMountItems.add(command); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java index 4f8844cba1..69530675e3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java @@ -25,7 +25,6 @@ import com.facebook.react.bridge.RetryableMountingLayerException; import com.facebook.react.bridge.SoftAssertions; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.ReactConstants; -import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener; import com.facebook.systrace.Systrace; @@ -596,7 +595,6 @@ public class UIViewOperationQueue { private final DispatchUIFrameCallback mDispatchUIFrameCallback; private final ReactApplicationContext mReactApplicationContext; - private final boolean mAllowViewCommandsQueue; private ArrayList mViewCommandOperations = new ArrayList<>(); // Only called from the UIManager queue? @@ -637,7 +635,6 @@ public class UIViewOperationQueue { ? DEFAULT_MIN_TIME_LEFT_IN_FRAME_FOR_NONBATCHED_OPERATION_MS : minTimeLeftInFrameForNonBatchedOperationMs); mReactApplicationContext = reactContext; - mAllowViewCommandsQueue = ReactFeatureFlags.allowEarlyViewCommandExecution; } /*package*/ NativeViewHierarchyManager getNativeViewHierarchyManager() { @@ -709,22 +706,14 @@ public class UIViewOperationQueue { int reactTag, int commandId, @Nullable ReadableArray commandArgs) { final DispatchCommandOperation command = new DispatchCommandOperation(reactTag, commandId, commandArgs); - if (mAllowViewCommandsQueue) { - mViewCommandOperations.add(command); - } else { - mOperations.add(command); - } + mViewCommandOperations.add(command); } public void enqueueDispatchCommand( int reactTag, String commandId, @Nullable ReadableArray commandArgs) { final DispatchStringCommandOperation command = new DispatchStringCommandOperation(reactTag, commandId, commandArgs); - if (mAllowViewCommandsQueue) { - mViewCommandOperations.add(command); - } else { - mOperations.add(command); - } + mViewCommandOperations.add(command); } public void enqueueUpdateExtraData(int reactTag, Object extraData) { From 8c14d518ce392805dc02bb2f0347c2a33fc1d771 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 29 Jun 2020 15:51:22 -0700 Subject: [PATCH 27/49] Fabric: Removing `const` qualifiers from some *Props fields Summary: Same as D19583582 (https://github.com/facebook/react-native/commit/5b2ea6ec6ab8cecad35d6309ceec3853aa37b0a3). We need this for implementing some tests. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22284646 fbshipit-source-id: 9dd6575475c133b9426838021af4d59fb9fc3c65 --- .../fabric/components/root/RootProps.h | 4 +- .../components/scrollview/ScrollViewProps.h | 52 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/ReactCommon/fabric/components/root/RootProps.h b/ReactCommon/fabric/components/root/RootProps.h index 11bda3e4c6..3cffe6256a 100644 --- a/ReactCommon/fabric/components/root/RootProps.h +++ b/ReactCommon/fabric/components/root/RootProps.h @@ -27,8 +27,8 @@ class RootProps final : public ViewProps { #pragma mark - Props - LayoutConstraints const layoutConstraints{}; - LayoutContext const layoutContext{}; + LayoutConstraints layoutConstraints{}; + LayoutContext layoutContext{}; }; } // namespace react diff --git a/ReactCommon/fabric/components/scrollview/ScrollViewProps.h b/ReactCommon/fabric/components/scrollview/ScrollViewProps.h index e520c17363..a6c08781f9 100644 --- a/ReactCommon/fabric/components/scrollview/ScrollViewProps.h +++ b/ReactCommon/fabric/components/scrollview/ScrollViewProps.h @@ -21,32 +21,32 @@ class ScrollViewProps final : public ViewProps { #pragma mark - Props - bool const alwaysBounceHorizontal{}; - bool const alwaysBounceVertical{}; - bool const bounces{true}; - bool const bouncesZoom{true}; - bool const canCancelContentTouches{true}; - bool const centerContent{}; - bool const automaticallyAdjustContentInsets{}; - Float const decelerationRate{0.998}; - bool const directionalLockEnabled{}; - ScrollViewIndicatorStyle const indicatorStyle{}; - ScrollViewKeyboardDismissMode const keyboardDismissMode{}; - Float const maximumZoomScale{1.0}; - Float const minimumZoomScale{1.0}; - bool const scrollEnabled{true}; - bool const pagingEnabled{}; - bool const pinchGestureEnabled{true}; - bool const scrollsToTop{true}; - bool const showsHorizontalScrollIndicator{true}; - bool const showsVerticalScrollIndicator{true}; - Float const scrollEventThrottle{}; - Float const zoomScale{1.0}; - EdgeInsets const contentInset{}; - Point const contentOffset{}; - EdgeInsets const scrollIndicatorInsets{}; - Float const snapToInterval{}; - ScrollViewSnapToAlignment const snapToAlignment{}; + bool alwaysBounceHorizontal{}; + bool alwaysBounceVertical{}; + bool bounces{true}; + bool bouncesZoom{true}; + bool canCancelContentTouches{true}; + bool centerContent{}; + bool automaticallyAdjustContentInsets{}; + Float decelerationRate{0.998}; + bool directionalLockEnabled{}; + ScrollViewIndicatorStyle indicatorStyle{}; + ScrollViewKeyboardDismissMode keyboardDismissMode{}; + Float maximumZoomScale{1.0}; + Float minimumZoomScale{1.0}; + bool scrollEnabled{true}; + bool pagingEnabled{}; + bool pinchGestureEnabled{true}; + bool scrollsToTop{true}; + bool showsHorizontalScrollIndicator{true}; + bool showsVerticalScrollIndicator{true}; + Float scrollEventThrottle{}; + Float zoomScale{1.0}; + EdgeInsets contentInset{}; + Point contentOffset{}; + EdgeInsets scrollIndicatorInsets{}; + Float snapToInterval{}; + ScrollViewSnapToAlignment snapToAlignment{}; #pragma mark - DebugStringConvertible From 559c60cf39a865b38fb4c22b9076a14e6655048b Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 29 Jun 2020 15:51:22 -0700 Subject: [PATCH 28/49] Fabric: Implementation of `insetBy(Rect, EdgeInsets) -> Rect` Summary: A function in graphics/geometry which adjusts a rectangle by the given edge insets. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22284644 fbshipit-source-id: ad78abd7889b4457c450b2cc43fb73054aef2c79 --- ReactCommon/fabric/graphics/Geometry.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ReactCommon/fabric/graphics/Geometry.h b/ReactCommon/fabric/graphics/Geometry.h index e1701ca311..36f71e8321 100644 --- a/ReactCommon/fabric/graphics/Geometry.h +++ b/ReactCommon/fabric/graphics/Geometry.h @@ -259,6 +259,15 @@ using EdgeInsets = RectangleEdges; */ using CornerInsets = RectangleCorners; +/* + * Adjusts a rectangle by the given edge insets. + */ +inline Rect insetBy(Rect rect, EdgeInsets insets) { + return Rect{{rect.origin.x + insets.left, rect.origin.y + insets.top}, + {rect.size.width - insets.left - insets.right, + rect.size.height - insets.top - insets.bottom}}; +} + } // namespace react } // namespace facebook From 8ceb8089373bcdac1a23470baa59defa9c133fe9 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Mon, 29 Jun 2020 15:51:22 -0700 Subject: [PATCH 29/49] Fabric: Removing `LayoutContext::absolutePosition` Summary: Here is why: * It was with us from the very beginning but we never use it. * The main purpose of this - snap-to-pixel layout - was moved to Yoga, where it should be. * The current implementation has a bug. * It's not really correct conceptually because the value becomes incorrect when an immutable subtree is being reused as part of a new tree. * It over-complicates a new feature I am working on. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22284645 fbshipit-source-id: c4c2df8d24e8fe924725b465e04e8154d097d226 --- React/Base/Surface/RCTSurfaceRootShadowView.m | 1 - .../fabric/components/text/paragraph/ParagraphShadowNode.cpp | 1 - ReactCommon/fabric/core/layout/LayoutContext.h | 5 ----- ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp | 3 --- 4 files changed, 10 deletions(-) diff --git a/React/Base/Surface/RCTSurfaceRootShadowView.m b/React/Base/Surface/RCTSurfaceRootShadowView.m index 97525cba47..455af97b1a 100644 --- a/React/Base/Surface/RCTSurfaceRootShadowView.m +++ b/React/Base/Surface/RCTSurfaceRootShadowView.m @@ -46,7 +46,6 @@ NSHashTable *other = [NSHashTable new]; RCTLayoutContext layoutContext = {}; - layoutContext.absolutePosition = CGPointZero; layoutContext.affectedShadowViews = affectedShadowViews; layoutContext.other = other; diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp index 7e1ffc641f..c81e1c1b4f 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp @@ -206,7 +206,6 @@ void ParagraphShadowNode::layout(LayoutContext layoutContext) { auto attachmentOrigin = roundToPixel<&round>( attachmentFrame.origin, layoutMetrics.pointScaleFactor); auto attachmentLayoutContext = layoutContext; - attachmentLayoutContext.absolutePosition += attachmentOrigin; auto attachmentLayoutConstrains = LayoutConstraints{ attachmentSize, attachmentSize, layoutConstraints.layoutDirection}; diff --git a/ReactCommon/fabric/core/layout/LayoutContext.h b/ReactCommon/fabric/core/layout/LayoutContext.h index c0b6ed0af4..42b011fca7 100644 --- a/ReactCommon/fabric/core/layout/LayoutContext.h +++ b/ReactCommon/fabric/core/layout/LayoutContext.h @@ -20,11 +20,6 @@ namespace react { * layout approaches. */ struct LayoutContext { - /* - * Compound absolute position of the node relative to the root node. - */ - Point absolutePosition{0, 0}; - /* * Reflects the scale factor needed to convert from the logical coordinate * space into the device coordinate space of the physical screen. diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp index 203fe8a2bb..35524cad1f 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp @@ -219,9 +219,6 @@ void LayoutableShadowNode::layout(LayoutContext layoutContext) { continue; } - auto childLayoutContext = LayoutContext(layoutContext); - childLayoutContext.absolutePosition += childLayoutMetrics.frame.origin; - child->layout(layoutContext); } } From 0fda91ffffa4972ebe58e3d0b610692a1286eaa1 Mon Sep 17 00:00:00 2001 From: fabriziobertoglio1987 Date: Mon, 29 Jun 2020 16:55:04 -0700 Subject: [PATCH 30/49] Adding Hyphenation Frequency prop for Text component (#29157) Summary: This issue fixes https://github.com/facebook/react-native/issues/28279 android_hyphenationFrequency prop for Android Text component which sets the frequency of automatic hyphenation to use when determining word breaks. ## Changelog [Android] [Fixed] - Adding Hyphenation Frequency prop for Text component Pull Request resolved: https://github.com/facebook/react-native/pull/29157 Test Plan: More info are available in the [android docs](https://developer.android.com/reference/android/widget/TextView#setHyphenationFrequency(int)). I will add the documentation to the docs later once the pull request is taken in consideration for merging. | **AFTER** | |:-------------------------:| | | I remain available to do improvements. Thanks a lot. Fabrizio. Reviewed By: TheSavior Differential Revision: D22219548 Pulled By: JoshuaGross fbshipit-source-id: 7e2523c25adfcd75454f60184eb73dc49891bef7 --- Libraries/Text/Text.js | 1 + Libraries/Text/TextProps.js | 12 ++++++++++ .../js/examples/Text/TextExample.android.js | 23 +++++++++++++++++++ .../text/ReactTextAnchorViewManager.java | 19 +++++++++++++++ 4 files changed, 55 insertions(+) diff --git a/Libraries/Text/Text.js b/Libraries/Text/Text.js index 660dd87976..6ff8c14c26 100644 --- a/Libraries/Text/Text.js +++ b/Libraries/Text/Text.js @@ -68,6 +68,7 @@ const viewConfig = { onTextLayout: true, onInlineViewLayout: true, dataDetectorType: true, + android_hyphenationFrequency: true, }, directEventTypes: { topTextLayout: { diff --git a/Libraries/Text/TextProps.js b/Libraries/Text/TextProps.js index 009929e8ff..4b48bc5706 100644 --- a/Libraries/Text/TextProps.js +++ b/Libraries/Text/TextProps.js @@ -57,6 +57,18 @@ export type TextProps = $ReadOnly<{| * See https://reactnative.dev/docs/text.html#allowfontscaling */ allowFontScaling?: ?boolean, + + /** + * Set hyphenation strategy on Android. + * + */ + android_hyphenationFrequency?: ?( + | 'normal' + | 'none' + | 'full' + | 'high' + | 'balanced' + ), children?: ?Node, /** diff --git a/RNTester/js/examples/Text/TextExample.android.js b/RNTester/js/examples/Text/TextExample.android.js index f9631821f9..cec4b348d1 100644 --- a/RNTester/js/examples/Text/TextExample.android.js +++ b/RNTester/js/examples/Text/TextExample.android.js @@ -146,6 +146,7 @@ class AdjustingFontSize extends React.Component< {'Multiline text component shrinking is supported, watch as this reeeeaaaally loooooong teeeeeeext grooooows and then shriiiinks as you add text to me! ioahsdia soady auydoa aoisyd aosdy ' + ' ' + @@ -207,6 +208,28 @@ class TextExample extends React.Component<{...}> { going to the next line. + + + Normal: + WillHaveAnHyphenWhenBreakingForNewLine + + + None: + WillNotHaveAnHyphenWhenBreakingForNewLine + + + Full: + WillHaveAnHyphenWhenBreakingForNewLine + + + High: + WillHaveAnHyphenWhenBreakingForNewLine + + + Balanced: + WillHaveAnHyphenWhenBreakingForNewLine + + This text is indented by 10px padding on all sides. diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java index 401c8a0fa3..4fdbd76aba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java @@ -7,6 +7,7 @@ package com.facebook.react.views.text; +import android.text.Layout; import android.text.Spannable; import android.text.TextUtils; import android.text.util.Linkify; @@ -96,6 +97,24 @@ public abstract class ReactTextAnchorViewManager Date: Mon, 29 Jun 2020 17:06:34 -0700 Subject: [PATCH 31/49] Fix: Preserve native animated value after animated component unmount (#28841) Summary: After animation has been finished using Native driver there is no final value passed from the native to JS side. This causes a bug from https://github.com/facebook/react-native/issues/28114. This PR solves this problem in the same way as `react-native-reanimated` library. When detaching it is calling native side to get the last value from Animated node and stores it on the JS side. Preserving animated value even if animation was using `useNativeDriver: true` Fixes https://github.com/facebook/react-native/issues/28114 ## Changelog [Internal] [Fixed] - Save native Animated node value on JS side in detach phase Pull Request resolved: https://github.com/facebook/react-native/pull/28841 Test Plan: Unit tests for added getValue method passed. Green CI Reviewed By: mdvacca Differential Revision: D22211499 Pulled By: JoshuaGross fbshipit-source-id: 9a3a98a9f9a8536fe2c8764f667cdabe1f6ba82a --- .../Animated/src/NativeAnimatedHelper.js | 9 ++ .../Animated/src/NativeAnimatedModule.js | 2 + .../src/__tests__/AnimatedNative-test.js | 21 ++++ Libraries/Animated/src/nodes/AnimatedValue.js | 5 + .../FBReactNativeSpec-generated.mm | 7 ++ .../FBReactNativeSpec/FBReactNativeSpec.h | 2 + .../RCTNativeAnimatedModule.mm | 7 +- .../RCTNativeAnimatedNodesManager.h | 3 + .../RCTNativeAnimatedNodesManager.m | 11 ++ .../RCTNativeAnimatedNodesManagerTests.m | 17 +++ .../js/examples/Animated/AnimatedExample.js | 109 ++++++++++++++++++ .../specs/NativeAnimatedModuleSpec.java | 3 + .../specs/jni/FBReactNativeSpec-generated.cpp | 7 ++ .../react/animated/NativeAnimatedModule.java | 12 ++ .../animated/NativeAnimatedNodesManager.java | 9 ++ .../NativeAnimatedNodeTraversalTest.java | 13 +++ 16 files changed, 236 insertions(+), 1 deletion(-) diff --git a/Libraries/Animated/src/NativeAnimatedHelper.js b/Libraries/Animated/src/NativeAnimatedHelper.js index 8cc6aa25b5..e24ba00097 100644 --- a/Libraries/Animated/src/NativeAnimatedHelper.js +++ b/Libraries/Animated/src/NativeAnimatedHelper.js @@ -38,6 +38,15 @@ const API = { enableQueue: function(): void { queueConnections = true; }, + getValue: function( + tag: number, + saveValueCallback: (value: number) => void, + ): void { + invariant(NativeAnimatedModule, 'Native animated module is not available'); + if (NativeAnimatedModule.getValue) { + NativeAnimatedModule.getValue(tag, saveValueCallback); + } + }, disableQueue: function(): void { invariant(NativeAnimatedModule, 'Native animated module is not available'); queueConnections = false; diff --git a/Libraries/Animated/src/NativeAnimatedModule.js b/Libraries/Animated/src/NativeAnimatedModule.js index fc76ec1953..51bb3e50d0 100644 --- a/Libraries/Animated/src/NativeAnimatedModule.js +++ b/Libraries/Animated/src/NativeAnimatedModule.js @@ -15,6 +15,7 @@ import * as TurboModuleRegistry from '../../TurboModule/TurboModuleRegistry'; type EndResult = {finished: boolean, ...}; type EndCallback = (result: EndResult) => void; +type SaveValueCallback = (value: number) => void; export type EventMapping = {| nativeEventPath: Array, @@ -28,6 +29,7 @@ export type AnimatingNodeConfig = Object; export interface Spec extends TurboModule { +createAnimatedNode: (tag: number, config: AnimatedNodeConfig) => void; + +getValue: (tag: number, saveValueCallback: SaveValueCallback) => void; +startListeningToAnimatedNodeValue: (tag: number) => void; +stopListeningToAnimatedNodeValue: (tag: number) => void; +connectAnimatedNodes: (parentTag: number, childTag: number) => void; diff --git a/Libraries/Animated/src/__tests__/AnimatedNative-test.js b/Libraries/Animated/src/__tests__/AnimatedNative-test.js index 4a68f78f3a..f2de0c3d4a 100644 --- a/Libraries/Animated/src/__tests__/AnimatedNative-test.js +++ b/Libraries/Animated/src/__tests__/AnimatedNative-test.js @@ -36,6 +36,7 @@ describe('Native Animated', () => { beforeEach(() => { Object.assign(NativeAnimatedModule, { + getValue: jest.fn(), addAnimatedEventToView: jest.fn(), connectAnimatedNodes: jest.fn(), connectAnimatedNodeToView: jest.fn(), @@ -115,6 +116,26 @@ describe('Native Animated', () => { ); }); + it('should save value on unmount', () => { + NativeAnimatedModule.getValue = jest.fn((tag, saveCallback) => { + saveCallback(1); + }); + const opacity = new Animated.Value(0); + + opacity.__makeNative(); + + const root = TestRenderer.create(); + const tag = opacity.__getNativeTag(); + + root.unmount(); + + expect(NativeAnimatedModule.getValue).toBeCalledWith( + tag, + expect.any(Function), + ); + expect(opacity.__getValue()).toBe(1); + }); + it('should extract offset', () => { const opacity = new Animated.Value(0); opacity.__makeNative(); diff --git a/Libraries/Animated/src/nodes/AnimatedValue.js b/Libraries/Animated/src/nodes/AnimatedValue.js index 7306fc77b9..3528e2fdbe 100644 --- a/Libraries/Animated/src/nodes/AnimatedValue.js +++ b/Libraries/Animated/src/nodes/AnimatedValue.js @@ -86,6 +86,11 @@ class AnimatedValue extends AnimatedWithChildren { } __detach() { + if (this.__isNative) { + NativeAnimatedAPI.getValue(this.__getNativeTag(), value => { + this._value = value; + }); + } this.stopAnimation(); super.__detach(); } diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm index 790d8fb1f8..6926dc825e 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm +++ b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm @@ -233,6 +233,10 @@ namespace facebook { return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "createAnimatedNode", @selector(createAnimatedNode:config:), args, count); } + static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_getValue(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { + return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "getValue", @selector(getValue:saveValueCallback:), args, count); + } + static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_startListeningToAnimatedNodeValue(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "startListeningToAnimatedNodeValue", @selector(startListeningToAnimatedNodeValue:), args, count); } @@ -312,6 +316,9 @@ namespace facebook { methodMap_["createAnimatedNode"] = MethodMetadata {2, __hostFunction_NativeAnimatedModuleSpecJSI_createAnimatedNode}; + methodMap_["getValue"] = MethodMetadata {2, __hostFunction_NativeAnimatedModuleSpecJSI_getValue}; + + methodMap_["startListeningToAnimatedNodeValue"] = MethodMetadata {1, __hostFunction_NativeAnimatedModuleSpecJSI_startListeningToAnimatedNodeValue}; diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h index cd228b2f43..1baf7d19f3 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h +++ b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h @@ -271,6 +271,8 @@ namespace JS { - (void)createAnimatedNode:(double)tag config:(NSDictionary *)config; +- (void)getValue:(double)tag +saveValueCallback:(RCTResponseSenderBlock)saveValueCallback; - (void)startListeningToAnimatedNodeValue:(double)tag; - (void)stopListeningToAnimatedNodeValue:(double)tag; - (void)connectAnimatedNodes:(double)parentTag diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm b/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm index a80f6b8824..2111c9b6c3 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm +++ b/Libraries/NativeAnimation/RCTNativeAnimatedModule.mm @@ -216,6 +216,12 @@ RCT_EXPORT_METHOD(removeAnimatedEventFromView:(double)viewTag }]; } +RCT_EXPORT_METHOD(getValue:(double)nodeTag saveCallback:(RCTResponseSenderBlock)saveCallback) { + [self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) { + [nodesManager getValue:[NSNumber numberWithDouble:nodeTag] saveCallback:saveCallback]; + }]; +} + #pragma mark -- Batch handling - (void)addOperationBlock:(AnimatedOperation)operation @@ -300,7 +306,6 @@ RCT_EXPORT_METHOD(removeAnimatedEventFromView:(double)viewTag operation(self->_nodesManager); } }]; - [uiManager addUIBlock:^(__unused RCTUIManager *manager, __unused NSDictionary *viewRegistry) { for (AnimatedOperation operation in operations) { operation(self->_nodesManager); diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h index ad9860147c..490117a885 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.h @@ -21,6 +21,9 @@ - (BOOL)isNodeManagedByFabric:(nonnull NSNumber *)tag; +- (void)getValue:(nonnull NSNumber *)nodeTag + saveCallback:(nullable RCTResponseSenderBlock)saveCallback; + // graph - (void)createAnimatedNode:(nonnull NSNumber *)tag diff --git a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m index 1c8a4b3e9f..265ff81d4e 100644 --- a/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m +++ b/Libraries/NativeAnimation/RCTNativeAnimatedNodesManager.m @@ -242,6 +242,17 @@ static NSString *RCTNormalizeAnimatedEventName(NSString *eventName) [valueNode extractOffset]; } +- (void)getValue:(NSNumber *)nodeTag saveCallback:(RCTResponseSenderBlock)saveCallback +{ + RCTAnimatedNode *node = _animationNodes[nodeTag]; + if (![node isKindOfClass:[RCTValueAnimatedNode class]]) { + RCTLogError(@"Not a value node."); + return; + } + RCTValueAnimatedNode *valueNode = (RCTValueAnimatedNode *)node;; + saveCallback(@[@(valueNode.value)]); +} + #pragma mark -- Drivers - (void)startAnimatingNode:(nonnull NSNumber *)animationId diff --git a/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m b/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m index 259c48a238..c5c599976e 100644 --- a/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m +++ b/RNTester/RNTesterUnitTests/RCTNativeAnimatedNodesManagerTests.m @@ -864,6 +864,23 @@ static id RCTPropChecker(NSString *prop, NSNumber *value) [_uiManager verify]; } +- (void) testGetValue +{ + __block NSInteger saveValueCallbackCalls = 0; + NSNumber *nodeTag = @100; + [_nodesManager createAnimatedNode:nodeTag + config:@{@"type": @"value", @"value": @1, @"offset": @0}]; + RCTResponseSenderBlock saveValueCallback = ^(NSArray *response) { + saveValueCallbackCalls++; + XCTAssertEqualObjects(response, @[@1]); + }; + + XCTAssertEqual(saveValueCallbackCalls, 0); + + [_nodesManager getValue:nodeTag saveCallback:saveValueCallback]; + XCTAssertEqual(saveValueCallbackCalls, 1); +} + /** * Creates a following graph of nodes: * Value(3, initialValue) ----> Style(4) ---> Props(5) ---> View(viewTag) diff --git a/RNTester/js/examples/Animated/AnimatedExample.js b/RNTester/js/examples/Animated/AnimatedExample.js index 7ef971ca5e..76c546dc9e 100644 --- a/RNTester/js/examples/Animated/AnimatedExample.js +++ b/RNTester/js/examples/Animated/AnimatedExample.js @@ -335,6 +335,115 @@ exports.examples = [ ); }, }, + { + title: 'Moving box example', + description: ('Click arrow buttons to move the box.' + + 'Then hide the box and reveal it again.' + + 'After that the box position will reset to initial position.': string), + render: (): React.Node => { + const containerWidth = 200; + const boxSize = 50; + + const movingBoxStyles = StyleSheet.create({ + container: { + display: 'flex', + alignItems: 'center', + flexDirection: 'column', + backgroundColor: '#fff', + padding: 30, + }, + boxContainer: { + backgroundColor: '#d3d3d3', + height: boxSize, + width: containerWidth, + }, + box: { + width: boxSize, + height: boxSize, + margin: 0, + }, + buttonsContainer: { + marginTop: 20, + display: 'flex', + flexDirection: 'row', + justifyContent: 'space-between', + width: containerWidth, + }, + }); + type Props = $ReadOnly<{||}>; + type State = {|boxVisible: boolean|}; + + class MovingBoxExample extends React.Component { + x: Animated.Value; + constructor(props) { + super(props); + this.x = new Animated.Value(0); + this.state = { + boxVisible: true, + }; + } + + render() { + const {boxVisible} = this.state; + const toggleText = boxVisible ? 'Hide' : 'Show'; + return ( + + {this.renderBox()} + + this.moveTo(0)}> + {'<-'} + + + {toggleText} + + this.moveTo(containerWidth - boxSize)}> + {'->'} + + + + ); + } + + renderBox = () => { + if (this.state.boxVisible) { + const horizontalLocation = {transform: [{translateX: this.x}]}; + return ( + + + + ); + } else { + return ( + + The box view is not being rendered + + ); + } + }; + + moveTo = x => { + Animated.timing(this.x, { + toValue: x, + duration: 1000, + useNativeDriver: true, + }).start(); + }; + + toggleVisibility = () => { + const {boxVisible} = this.state; + this.setState({boxVisible: !boxVisible}); + }; + } + return ; + }, + }, { title: 'Continuous Interactions', description: ('Gesture events, chaining, 2D ' + diff --git a/ReactAndroid/src/main/java/com/facebook/fbreact/specs/NativeAnimatedModuleSpec.java b/ReactAndroid/src/main/java/com/facebook/fbreact/specs/NativeAnimatedModuleSpec.java index 91710dbc5e..0bfc02aaaf 100644 --- a/ReactAndroid/src/main/java/com/facebook/fbreact/specs/NativeAnimatedModuleSpec.java +++ b/ReactAndroid/src/main/java/com/facebook/fbreact/specs/NativeAnimatedModuleSpec.java @@ -66,6 +66,9 @@ public abstract class NativeAnimatedModuleSpec extends ReactContextBaseJavaModul public abstract void startAnimatingNode(double animationId, double nodeTag, ReadableMap config, Callback endCallback); + @ReactMethod + public abstract void getValue(double tag, Callback saveValueCallback); + @ReactMethod public abstract void stopListeningToAnimatedNodeValue(double tag); diff --git a/ReactAndroid/src/main/java/com/facebook/fbreact/specs/jni/FBReactNativeSpec-generated.cpp b/ReactAndroid/src/main/java/com/facebook/fbreact/specs/jni/FBReactNativeSpec-generated.cpp index 2283e38b31..c8f70598ae 100644 --- a/ReactAndroid/src/main/java/com/facebook/fbreact/specs/jni/FBReactNativeSpec-generated.cpp +++ b/ReactAndroid/src/main/java/com/facebook/fbreact/specs/jni/FBReactNativeSpec-generated.cpp @@ -186,6 +186,10 @@ namespace facebook { return static_cast(turboModule).invokeJavaMethod(rt, VoidKind, "createAnimatedNode", "(DLcom/facebook/react/bridge/ReadableMap;)V", args, count); } + static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_getValue(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { + return static_cast(turboModule).invokeJavaMethod(rt, VoidKind, "getValue", "(DLcom/facebook/react/bridge/Callback;)V", args, count); + } + static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_startListeningToAnimatedNodeValue(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { return static_cast(turboModule).invokeJavaMethod(rt, VoidKind, "startListeningToAnimatedNodeValue", "(D)V", args, count); } @@ -265,6 +269,9 @@ namespace facebook { methodMap_["createAnimatedNode"] = MethodMetadata {2, __hostFunction_NativeAnimatedModuleSpecJSI_createAnimatedNode}; + methodMap_["getValue"] = MethodMetadata {2, __hostFunction_NativeAnimatedModuleSpecJSI_getValue}; + + methodMap_["startListeningToAnimatedNodeValue"] = MethodMetadata {1, __hostFunction_NativeAnimatedModuleSpecJSI_startListeningToAnimatedNodeValue}; diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index fc3c4efb89..f85a092952 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -774,4 +774,16 @@ public class NativeAnimatedModule extends NativeAnimatedModuleSpec public void removeListeners(double count) { // iOS only } + + @Override + public void getValue(final double animatedValueNodeTagDouble, final Callback callback) { + final int animatedValueNodeTag = (int) animatedValueNodeTagDouble; + mOperations.add( + new UIThreadOperation() { + @Override + public void execute(NativeAnimatedNodesManager animatedNodesManager) { + animatedNodesManager.getValue(animatedValueNodeTag, callback); + } + }); + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 0156393d31..f1c76b780d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -352,6 +352,15 @@ import java.util.Queue; propsAnimatedNode.disconnectFromView(viewTag); } + public void getValue(int tag, Callback callback) { + AnimatedNode node = mAnimatedNodes.get(tag); + if (node == null || !(node instanceof ValueAnimatedNode)) { + throw new JSApplicationIllegalArgumentException( + "Animated node with tag " + tag + " does not exists or is not a 'value' node"); + } + callback.invoke(((ValueAnimatedNode) node).getValue()); + } + public void restoreDefaultValues(int animatedNodeTag) { AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); // Restoring default values needs to happen before UIManager operations so it is diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java index 4c675ee66e..30a9a3bc41 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java @@ -860,6 +860,19 @@ public class NativeAnimatedNodeTraversalTest { verifyNoMoreInteractions(animationCallback); } + @Test + public void testGetValue() { + int tag = 1; + mNativeAnimatedNodesManager.createAnimatedNode( + tag, JavaOnlyMap.of("type", "value", "value", 1d, "offset", 0d)); + + Callback saveValueCallbackMock = mock(Callback.class); + + mNativeAnimatedNodesManager.getValue(tag, saveValueCallbackMock); + + verify(saveValueCallbackMock, times(1)).invoke(1d); + } + @Test public void testInterpolationNode() { mNativeAnimatedNodesManager.createAnimatedNode( From 23fbfc1977fe9e9b1feaa08d70e3be5bbf0d1aa4 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 29 Jun 2020 18:19:50 -0700 Subject: [PATCH 32/49] Remove SectionList hack that causes Fabric crash Summary: View elevation is an Android-only prop, and it's a float, not an int. https://www.codota.com/code/java/methods/android.view.View/setElevation Changelog: [Internal] Reviewed By: shergin Differential Revision: D22298431 fbshipit-source-id: 91edc3aa64e7254a5c4a4cb33f26e64ebae89b8d --- ReactCommon/fabric/components/view/ViewProps.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactCommon/fabric/components/view/ViewProps.h b/ReactCommon/fabric/components/view/ViewProps.h index 95730676ca..6d00421341 100644 --- a/ReactCommon/fabric/components/view/ViewProps.h +++ b/ReactCommon/fabric/components/view/ViewProps.h @@ -59,7 +59,7 @@ class ViewProps : public YogaStylableProps, public AccessibilityProps { bool collapsable{true}; - int elevation{}; + Float elevation{}; /* Android-only */ #pragma mark - Convenience Methods From 3d4535a2bb0d9df6c50f7504512f9ceefe6c56ec Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 30 Jun 2020 01:33:52 -0700 Subject: [PATCH 33/49] Introduce InputAccessoryView Summary: Changelog: [Internal] Introducing InputAccessoryView. There is one big difference between Fabric's implementation and Paper's implementation. Fabric searches for text input from InputAccessoryView, unlike Paper where it is the other way around. Reviewed By: shergin Differential Revision: D22160445 fbshipit-source-id: 55313fe50afeced7aead5b57137d711dd1cfd3ae --- .../RCTInputAccessoryViewNativeComponent.js | 8 +- .../Text/TextInput/Multiline/RCTUITextView.h | 2 + .../RCTBackedTextInputViewProtocol.h | 1 + .../TextInput/Singleline/RCTUITextField.h | 1 + .../RCTInputAccessoryComponentView.h | 21 +++ .../RCTInputAccessoryComponentView.mm | 153 ++++++++++++++++++ .../RCTInputAccessoryContentView.h | 12 ++ .../RCTInputAccessoryContentView.mm | 70 ++++++++ .../RCTFabricComponentsPlugins.h | 1 + .../RCTFabricComponentsPlugins.mm | 1 + .../TextInput/RCTTextInputComponentView.mm | 4 + .../fabric/components/inputaccessory/BUCK | 47 ++++++ .../InputAccessoryComponentDescriptor.h | 48 ++++++ .../InputAccessoryShadowNode.cpp | 16 ++ .../inputaccessory/InputAccessoryShadowNode.h | 39 +++++ .../inputaccessory/InputAccessoryState.h | 29 ++++ .../textinput/iostextinput/TextInputProps.cpp | 7 +- .../textinput/iostextinput/TextInputProps.h | 2 + 18 files changed, 458 insertions(+), 4 deletions(-) create mode 100644 React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.h create mode 100644 React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.mm create mode 100644 React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.h create mode 100644 React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.mm create mode 100644 ReactCommon/fabric/components/inputaccessory/BUCK create mode 100644 ReactCommon/fabric/components/inputaccessory/InputAccessoryComponentDescriptor.h create mode 100644 ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.cpp create mode 100644 ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.h create mode 100644 ReactCommon/fabric/components/inputaccessory/InputAccessoryState.h diff --git a/Libraries/Components/TextInput/RCTInputAccessoryViewNativeComponent.js b/Libraries/Components/TextInput/RCTInputAccessoryViewNativeComponent.js index 7fc7ed4861..6120e1614c 100644 --- a/Libraries/Components/TextInput/RCTInputAccessoryViewNativeComponent.js +++ b/Libraries/Components/TextInput/RCTInputAccessoryViewNativeComponent.js @@ -21,6 +21,8 @@ type NativeProps = $ReadOnly<{| backgroundColor?: ?ColorValue, |}>; -export default (codegenNativeComponent( - 'RCTInputAccessoryView', -): HostComponent); +export default (codegenNativeComponent('InputAccessory', { + interfaceOnly: true, + paperComponentName: 'RCTInputAccessoryView', + excludedPlatforms: ['android'], +}): HostComponent); diff --git a/Libraries/Text/TextInput/Multiline/RCTUITextView.h b/Libraries/Text/TextInput/Multiline/RCTUITextView.h index 7e12add51d..8f4526bd40 100644 --- a/Libraries/Text/TextInput/Multiline/RCTUITextView.h +++ b/Libraries/Text/TextInput/Multiline/RCTUITextView.h @@ -37,6 +37,8 @@ NS_ASSUME_NONNULL_BEGIN // it's declared here only to conform to the interface. @property (nonatomic, assign) BOOL caretHidden; +@property (nonatomic, strong, nullable) NSString *inputAccessoryViewID; + @end NS_ASSUME_NONNULL_END diff --git a/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h b/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h index 2fd80897eb..09f4fe2766 100644 --- a/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h +++ b/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h @@ -30,6 +30,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign) BOOL enablesReturnKeyAutomatically; @property (nonatomic, assign) UITextFieldViewMode clearButtonMode; @property (nonatomic, getter=isScrollEnabled) BOOL scrollEnabled; +@property (nonatomic, strong, nullable) NSString *inputAccessoryViewID; // This protocol disallows direct access to `selectedTextRange` property because // unwise usage of it can break the `delegate` behavior. So, we always have to diff --git a/Libraries/Text/TextInput/Singleline/RCTUITextField.h b/Libraries/Text/TextInput/Singleline/RCTUITextField.h index 7e03154db5..f3b3492607 100644 --- a/Libraries/Text/TextInput/Singleline/RCTUITextField.h +++ b/Libraries/Text/TextInput/Singleline/RCTUITextField.h @@ -28,6 +28,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign) UIEdgeInsets textContainerInset; @property (nonatomic, assign, getter=isEditable) BOOL editable; @property (nonatomic, getter=isScrollEnabled) BOOL scrollEnabled; +@property (nonatomic, strong, nullable) NSString *inputAccessoryViewID; @end diff --git a/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.h b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.h new file mode 100644 index 0000000000..37abe305e0 --- /dev/null +++ b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +#import + +NS_ASSUME_NONNULL_BEGIN + +/** + * UIView class for root component. + */ +@interface RCTInputAccessoryComponentView : RCTViewComponentView + +@end + +NS_ASSUME_NONNULL_END diff --git a/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.mm b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.mm new file mode 100644 index 0000000000..cf24358e27 --- /dev/null +++ b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryComponentView.mm @@ -0,0 +1,153 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "RCTInputAccessoryComponentView.h" + +#import +#import +#import +#import +#import +#import +#import "RCTInputAccessoryContentView.h" + +#import "RCTFabricComponentsPlugins.h" + +using namespace facebook::react; + +static UIView *_Nullable RCTFindTextInputWithNativeId(UIView *view, NSString *nativeId) +{ + if ([view respondsToSelector:@selector(inputAccessoryViewID)] && + [view respondsToSelector:@selector(setInputAccessoryView:)]) { + UIView *typed = (UIView *)view; + if (!nativeId || [typed.inputAccessoryViewID isEqualToString:nativeId]) { + return typed; + } + } + + for (UIView *subview in view.subviews) { + UIView *result = RCTFindTextInputWithNativeId(subview, nativeId); + if (result) { + return result; + } + } + + return nil; +} + +@implementation RCTInputAccessoryComponentView { + InputAccessoryShadowNode::ConcreteState::Shared _state; + RCTInputAccessoryContentView *_contentView; + RCTSurfaceTouchHandler *_touchHandler; + UIView __weak *_textInput; +} + +- (instancetype)initWithFrame:(CGRect)frame +{ + if (self = [super initWithFrame:frame]) { + static const auto defaultProps = std::make_shared(); + _props = defaultProps; + _contentView = [RCTInputAccessoryContentView new]; + _touchHandler = [RCTSurfaceTouchHandler new]; + [_touchHandler attachToView:_contentView]; + } + + return self; +} + +- (void)didMoveToWindow +{ + [super didMoveToWindow]; + + if (self.window && !_textInput) { + if (self.nativeId) { + _textInput = RCTFindTextInputWithNativeId(self.window, self.nativeId); + _textInput.inputAccessoryView = _contentView; + } else { + _textInput = RCTFindTextInputWithNativeId(_contentView, nil); + } + + if (!self.nativeId) { + [self becomeFirstResponder]; + } + } +} + +- (BOOL)canBecomeFirstResponder +{ + return true; +} + +- (UIView *)inputAccessoryView +{ + return _contentView; +} + +#pragma mark - RCTComponentViewProtocol + ++ (ComponentDescriptorProvider)componentDescriptorProvider +{ + return concreteComponentDescriptorProvider(); +} + +- (void)mountChildComponentView:(UIView *)childComponentView index:(NSInteger)index +{ + [_contentView insertSubview:childComponentView atIndex:index]; +} + +- (void)unmountChildComponentView:(UIView *)childComponentView index:(NSInteger)index +{ + [childComponentView removeFromSuperview]; +} + +- (void)updateProps:(Props::Shared const &)props oldProps:(Props::Shared const &)oldProps +{ + auto const &oldInputAccessoryProps = *std::static_pointer_cast(_props); + auto const &newInputAccessoryProps = *std::static_pointer_cast(props); + + if (newInputAccessoryProps.backgroundColor != oldInputAccessoryProps.backgroundColor) { + _contentView.backgroundColor = RCTUIColorFromSharedColor(newInputAccessoryProps.backgroundColor); + } + + [super updateProps:props oldProps:oldProps]; + self.hidden = true; +} + +- (void)updateState:(const facebook::react::State::Shared &)state + oldState:(const facebook::react::State::Shared &)oldState +{ + _state = std::static_pointer_cast(state); + CGSize oldScreenSize = RCTCGSizeFromSize(_state->getData().screenSize); + CGSize screenSize = [[UIScreen mainScreen] bounds].size; + screenSize.height = std::nan(""); + if (oldScreenSize.width != screenSize.width) { + auto stateData = InputAccessoryState{RCTSizeFromCGSize(screenSize)}; + _state->updateState(std::move(stateData)); + } +} + +- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics + oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics +{ + [super updateLayoutMetrics:layoutMetrics oldLayoutMetrics:oldLayoutMetrics]; + + [_contentView setFrame:RCTCGRectFromRect(layoutMetrics.getContentFrame())]; +} + +- (void)prepareForRecycle +{ + [super prepareForRecycle]; + _state.reset(); + _textInput = nil; +} + +@end + +Class RCTInputAccessoryCls(void) +{ + return RCTInputAccessoryComponentView.class; +} diff --git a/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.h b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.h new file mode 100644 index 0000000000..15c80e6a02 --- /dev/null +++ b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.h @@ -0,0 +1,12 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import + +@interface RCTInputAccessoryContentView : UIView + +@end diff --git a/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.mm b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.mm new file mode 100644 index 0000000000..d9c4fb231b --- /dev/null +++ b/React/Fabric/Mounting/ComponentViews/InputAccessory/RCTInputAccessoryContentView.mm @@ -0,0 +1,70 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#import "RCTInputAccessoryContentView.h" + +@implementation RCTInputAccessoryContentView { + UIView *_safeAreaContainer; + NSLayoutConstraint *_heightConstraint; +} + +- (instancetype)init +{ + if (self = [super init]) { + self.autoresizingMask = UIViewAutoresizingFlexibleHeight; + + _safeAreaContainer = [UIView new]; + _safeAreaContainer.translatesAutoresizingMaskIntoConstraints = NO; + [self addSubview:_safeAreaContainer]; + + _heightConstraint = [_safeAreaContainer.heightAnchor constraintEqualToConstant:0]; + _heightConstraint.active = YES; + + if (@available(iOS 11.0, tvOS 11.0, *)) { + [NSLayoutConstraint activateConstraints:@[ + [_safeAreaContainer.bottomAnchor constraintEqualToAnchor:self.safeAreaLayoutGuide.bottomAnchor], + [_safeAreaContainer.topAnchor constraintEqualToAnchor:self.safeAreaLayoutGuide.topAnchor], + [_safeAreaContainer.leadingAnchor constraintEqualToAnchor:self.safeAreaLayoutGuide.leadingAnchor], + [_safeAreaContainer.trailingAnchor constraintEqualToAnchor:self.safeAreaLayoutGuide.trailingAnchor] + ]]; + } else { + [NSLayoutConstraint activateConstraints:@[ + [_safeAreaContainer.bottomAnchor constraintEqualToAnchor:self.bottomAnchor], + [_safeAreaContainer.topAnchor constraintEqualToAnchor:self.topAnchor], + [_safeAreaContainer.leadingAnchor constraintEqualToAnchor:self.leadingAnchor], + [_safeAreaContainer.trailingAnchor constraintEqualToAnchor:self.trailingAnchor] + ]]; + } + } + return self; +} + +- (CGSize)intrinsicContentSize +{ + // This is needed so the view size is based on autolayout constraints. + return CGSizeZero; +} + +- (void)insertSubview:(UIView *)view atIndex:(NSInteger)index +{ + [_safeAreaContainer insertSubview:view atIndex:index]; +} + +- (void)setFrame:(CGRect)frame +{ + [super setFrame:frame]; + [_safeAreaContainer setFrame:frame]; + _heightConstraint.constant = frame.size.height; + [self layoutIfNeeded]; +} + +- (BOOL)canBecomeFirstResponder +{ + return true; +} + +@end diff --git a/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h b/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h index 7b002cbf8b..573e7b1fc6 100644 --- a/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h +++ b/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.h @@ -40,6 +40,7 @@ Class RCTModalHostViewCls(void) __attribute__((used)); Class RCTImageCls(void) __attribute__((used)); Class RCTParagraphCls(void) __attribute__((used)); Class RCTTextInputCls(void) __attribute__((used)); +Class RCTInputAccessoryCls(void) __attribute__((used)); Class RCTViewCls(void) __attribute__((used)); #ifdef __cplusplus diff --git a/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm b/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm index 33cc39126f..6107f1297f 100644 --- a/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm +++ b/React/Fabric/Mounting/ComponentViews/RCTFabricComponentsPlugins.mm @@ -29,6 +29,7 @@ Class RCTFabricComponentsProvider(const char *name) { {"Image", RCTImageCls}, {"Paragraph", RCTParagraphCls}, {"TextInput", RCTTextInputCls}, + {"InputAccessoryView", RCTInputAccessoryCls}, {"View", RCTViewCls}, }; diff --git a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm index 3bf312aa25..f23be7a946 100644 --- a/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm @@ -207,6 +207,10 @@ using namespace facebook::react; _backedTextInputView.tintColor = RCTUIColorFromSharedColor(newTextInputProps.selectionColor); } + if (newTextInputProps.inputAccessoryViewID != oldTextInputProps.inputAccessoryViewID) { + _backedTextInputView.inputAccessoryViewID = RCTNSStringFromString(newTextInputProps.inputAccessoryViewID); + } + [super updateProps:props oldProps:oldProps]; } diff --git a/ReactCommon/fabric/components/inputaccessory/BUCK b/ReactCommon/fabric/components/inputaccessory/BUCK new file mode 100644 index 0000000000..7db6c8ae3e --- /dev/null +++ b/ReactCommon/fabric/components/inputaccessory/BUCK @@ -0,0 +1,47 @@ +load("@fbsource//tools/build_defs/apple:flag_defs.bzl", "get_preprocessor_flags_for_build_mode") +load( + "//tools/build_defs/oss:rn_defs.bzl", + "APPLE", + "get_apple_compiler_flags", + "get_apple_inspector_flags", + "react_native_xplat_target", + "rn_xplat_cxx_library", + "subdir_glob", +) + +APPLE_COMPILER_FLAGS = get_apple_compiler_flags() + +rn_xplat_cxx_library( + name = "inputaccessory", + srcs = glob( + ["**/*.cpp"], + ), + headers = [], + header_namespace = "", + exported_headers = subdir_glob( + [ + ("", "*.h"), + ], + prefix = "react/components/inputaccessory", + ), + compiler_flags = [ + "-fexceptions", + "-frtti", + "-std=c++14", + "-Wall", + ], + fbobjc_compiler_flags = APPLE_COMPILER_FLAGS, + fbobjc_preprocessor_flags = get_preprocessor_flags_for_build_mode() + get_apple_inspector_flags(), + force_static = True, + labels = ["supermodule:xplat/default/public.react_native.infra"], + platforms = (APPLE), + preprocessor_flags = [ + "-DLOG_TAG=\"ReactNative\"", + "-DWITH_FBSYSTRACE=1", + ], + visibility = ["PUBLIC"], + deps = [ + react_native_xplat_target("fabric/core:core"), + "//xplat/js/react-native-github:generated_components-rncore", + ], +) diff --git a/ReactCommon/fabric/components/inputaccessory/InputAccessoryComponentDescriptor.h b/ReactCommon/fabric/components/inputaccessory/InputAccessoryComponentDescriptor.h new file mode 100644 index 0000000000..a850b1639a --- /dev/null +++ b/ReactCommon/fabric/components/inputaccessory/InputAccessoryComponentDescriptor.h @@ -0,0 +1,48 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook { +namespace react { + +/* + * Descriptor for component. + */ +class InputAccessoryComponentDescriptor final + : public ConcreteComponentDescriptor { + public: + using ConcreteComponentDescriptor::ConcreteComponentDescriptor; + + void adopt(UnsharedShadowNode shadowNode) const override { + assert(std::dynamic_pointer_cast(shadowNode)); + auto concreteShadowNode = + std::static_pointer_cast(shadowNode); + + assert(std::dynamic_pointer_cast( + concreteShadowNode)); + auto layoutableShadowNode = + std::static_pointer_cast(concreteShadowNode); + + auto state = + std::static_pointer_cast( + shadowNode->getState()); + auto stateData = state->getData(); + + layoutableShadowNode->setSize( + Size{stateData.screenSize.width, stateData.screenSize.height}); + layoutableShadowNode->setPositionType(YGPositionTypeAbsolute); + + ConcreteComponentDescriptor::adopt(shadowNode); + } +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.cpp b/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.cpp new file mode 100644 index 0000000000..dcc59dcb02 --- /dev/null +++ b/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.cpp @@ -0,0 +1,16 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "InputAccessoryShadowNode.h" + +namespace facebook { +namespace react { + +extern const char InputAccessoryComponentName[] = "InputAccessoryView"; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.h b/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.h new file mode 100644 index 0000000000..5a7f6679c7 --- /dev/null +++ b/ReactCommon/fabric/components/inputaccessory/InputAccessoryShadowNode.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include +#include + +namespace facebook { +namespace react { + +extern const char InputAccessoryComponentName[]; + +/* + * `ShadowNode` for component. + */ +class InputAccessoryShadowNode final : public ConcreteViewShadowNode< + InputAccessoryComponentName, + InputAccessoryProps, + InputAccessoryEventEmitter, + InputAccessoryState> { + public: + using ConcreteViewShadowNode::ConcreteViewShadowNode; + + static ShadowNodeTraits BaseTraits() { + auto traits = ConcreteViewShadowNode::BaseTraits(); + traits.set(ShadowNodeTraits::Trait::RootNodeKind); + return traits; + } +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/components/inputaccessory/InputAccessoryState.h b/ReactCommon/fabric/components/inputaccessory/InputAccessoryState.h new file mode 100644 index 0000000000..5d1ae67a63 --- /dev/null +++ b/ReactCommon/fabric/components/inputaccessory/InputAccessoryState.h @@ -0,0 +1,29 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include +#include + +namespace facebook { +namespace react { + +/* + * State for component. + */ +class InputAccessoryState final { + public: + InputAccessoryState(){}; + InputAccessoryState(Size screenSize_) : screenSize(screenSize_){}; + + const Size screenSize{}; +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.cpp b/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.cpp index f81a8c8fe2..972c3768e0 100644 --- a/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.cpp +++ b/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.cpp @@ -56,7 +56,12 @@ TextInputProps::TextInputProps( sourceProps.mostRecentEventCount, {})), autoFocus( - convertRawProp(rawProps, "autoFocus", sourceProps.autoFocus, {})){}; + convertRawProp(rawProps, "autoFocus", sourceProps.autoFocus, {})), + inputAccessoryViewID(convertRawProp( + rawProps, + "inputAccessoryViewID", + sourceProps.inputAccessoryViewID, + {})){}; TextAttributes TextInputProps::getEffectiveTextAttributes( Float fontSizeMultiplier) const { diff --git a/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.h b/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.h index 328e926b73..9109a2ae49 100644 --- a/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.h +++ b/ReactCommon/fabric/components/textinput/iostextinput/TextInputProps.h @@ -55,6 +55,8 @@ class TextInputProps final : public ViewProps, public BaseTextProps { bool autoFocus{false}; + std::string const inputAccessoryViewID{}; + /* * Accessors */ From d3c1011c0992b37eb74f5ba182fb94966a12c8f7 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 30 Jun 2020 01:44:47 -0700 Subject: [PATCH 34/49] Fabric: `LayoutableShadowNode::layoutChildren()` was merged into `layout()` Summary: After years of trying to design a somehow decomposed and universal layout API, I think we chased a wrong goal. None of the solutions that I tried helped with simplicity, composability, performance, or readability. Besides that, a bunch of execution primitives we use (e.g. `setHasNewLayout`) are heavily influenced by Yoga and are far from being commonly applicable. Finally, we ended up with a situation where the current design does not allow us to implement the features we want (more about that in coming diffs). The diff changes that. Now we have only one method that implements layout - `layout()`; all possible implementations are free to implement it in any way that makes sense for a particular approach. And, I think, even for the base case - Yoga powered layout - things are much simpler and faster now: it's easy to comprehend how a single method works and now we don't have two loops and an expensive call to `getLayoutableChildNodes`. But most importantly, after this change will be able to implement the `Inset Overflow` feature quite easily. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D22297317 fbshipit-source-id: f8662b7e8e3b522bdd6e5b2b0ac6a06efb45be6d --- .../view/yoga/YogaLayoutableShadowNode.cpp | 51 +++++++++++-------- .../view/yoga/YogaLayoutableShadowNode.h | 2 +- .../core/layout/LayoutableShadowNode.cpp | 22 +------- .../fabric/core/layout/LayoutableShadowNode.h | 16 +++--- 4 files changed, 38 insertions(+), 53 deletions(-) diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 99fd2a04c9..4094b390a2 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -352,35 +352,44 @@ void YogaLayoutableShadowNode::layoutTree( layout(layoutContext); } -void YogaLayoutableShadowNode::layoutChildren(LayoutContext layoutContext) { +void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { + // Reading data from a dirtied node does not make sense. assert(!yogaNode_.isDirty()); - for (const auto &childYogaNode : yogaNode_.getChildren()) { - if (!childYogaNode->getHasNewLayout()) { - continue; - } - - assert(!childYogaNode->isDirty()); - - auto childNode = - static_cast(childYogaNode->getContext()); + for (auto childYogaNode : yogaNode_.getChildren()) { + auto &childNode = + *static_cast(childYogaNode->getContext()); // Verifying that the Yoga node belongs to the ShadowNode. - assert(&childNode->yogaNode_ == childYogaNode); + assert(&childNode.yogaNode_ == childYogaNode); - LayoutMetrics childLayoutMetrics = - layoutMetricsFromYogaNode(childNode->yogaNode_); - childLayoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor; + if (childYogaNode->getHasNewLayout()) { + childYogaNode->setHasNewLayout(false); - // We must copy layout metrics from Yoga node only once (when the parent - // node exclusively ownes the child node). - assert(childYogaNode->getOwner() == &yogaNode_); + // Reading data from a dirtied node does not make sense. + assert(!childYogaNode->isDirty()); - childNode->ensureUnsealed(); - auto affected = childNode->setLayoutMetrics(childLayoutMetrics); + // We must copy layout metrics from Yoga node only once (when the parent + // node exclusively ownes the child node). + assert(childYogaNode->getOwner() == &yogaNode_); - if (affected && layoutContext.affectedNodes) { - layoutContext.affectedNodes->push_back(childNode); + // We are about to mutate layout metrics of the node. + childNode.ensureUnsealed(); + + auto newLayoutMetrics = layoutMetricsFromYogaNode(*childYogaNode); + newLayoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor; + + // Adding the node to `affectedNodes` if the node's `frame` was changed. + if (layoutContext.affectedNodes && + newLayoutMetrics.frame != childNode.getLayoutMetrics().frame) { + layoutContext.affectedNodes->push_back(&childNode); + } + + childNode.setLayoutMetrics(newLayoutMetrics); + + if (newLayoutMetrics.displayType != DisplayType::None) { + childNode.layout(layoutContext); + } } } } diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index 59148e333b..5372e8274c 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -84,7 +84,7 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { LayoutContext layoutContext, LayoutConstraints layoutConstraints) override; - void layoutChildren(LayoutContext layoutContext) override; + void layout(LayoutContext layoutContext) override; protected: /* diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp index 35524cad1f..b7f0458382 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp @@ -204,23 +204,7 @@ void LayoutableShadowNode::layoutTree( } void LayoutableShadowNode::layout(LayoutContext layoutContext) { - layoutChildren(layoutContext); - - for (auto child : getLayoutableChildNodes()) { - if (!child->getHasNewLayout()) { - continue; - } - - child->ensureUnsealed(); - child->setHasNewLayout(false); - - auto childLayoutMetrics = child->getLayoutMetrics(); - if (childLayoutMetrics.displayType == DisplayType::None) { - continue; - } - - child->layout(layoutContext); - } + // Default implementation does nothing. } ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint( @@ -250,10 +234,6 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint( return isPointInside ? node : nullptr; } -void LayoutableShadowNode::layoutChildren(LayoutContext layoutContext) { - // Default implementation does nothing. -} - #if RN_DEBUG_STRING_CONVERTIBLE SharedDebugStringConvertibleList LayoutableShadowNode::getDebugProps() const { auto list = SharedDebugStringConvertibleList{}; diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h index ed9c115dc5..5882b3a78c 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h @@ -93,9 +93,12 @@ class LayoutableShadowNode : public ShadowNode { * Computes layout recursively. * Additional environmental constraints might be provided via `layoutContext` * argument. - * Default implementation basically calls `layoutChildren()` and then - * `layout()` (recursively), and provides some obvious performance - * optimization. + * + * The typical concrete-layout-specific implementation of this method should: + * - Measure children with `LayoutConstraints` calculated from its size using + * a particular layout approach; + * - Calculate and assign `LayoutMetrics` for the children; + * - Call itself recursively on every child if needed. */ virtual void layout(LayoutContext layoutContext); @@ -158,12 +161,6 @@ class LayoutableShadowNode : public ShadowNode { virtual void setHasNewLayout(bool hasNewLayout) = 0; virtual bool getHasNewLayout() const = 0; - /* - * Applies layout for all children; - * does not call anything in recursive manner *by desing*. - */ - virtual void layoutChildren(LayoutContext layoutContext); - /* * Unifed methods to access text layout metrics. */ @@ -181,7 +178,6 @@ class LayoutableShadowNode : public ShadowNode { SharedDebugStringConvertibleList getDebugProps() const; #endif - private: LayoutMetrics layoutMetrics_; }; From 0471f90f1690e91a5b2c87056713924357a96267 Mon Sep 17 00:00:00 2001 From: Lulu Wu Date: Tue, 30 Jun 2020 06:38:02 -0700 Subject: [PATCH 35/49] Change onAssetDidLoad to bubbling event Summary: onAssetDidLoad is defined as bubbling event in iOS, change it to bubbling event and adde it to bubblingEventTypes to fix "missing: topAssetDidLoad" error in Viewpoints iOS. Changelog: [Internal] Reviewed By: JoshuaGross, mdvacca Differential Revision: D22209406 fbshipit-source-id: 5da779d9d1d62c70d85d84ad80807ff688e29e2f --- .../Components/View/ReactNativeViewViewConfigAndroid.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js b/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js index 6f4056a0e7..ad2542dfa2 100644 --- a/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js +++ b/Libraries/Components/View/ReactNativeViewViewConfigAndroid.js @@ -19,6 +19,12 @@ const ReactNativeViewViewConfigAndroid = { captured: 'onSelectCapture', }, }, + topAssetDidLoad: { + phasedRegistrationNames: { + bubbled: 'onAssetDidLoad', + captured: 'onAssetDidLoadCapture', + }, + }, }, directEventTypes: { topClick: { From 83d7fbcf9492bc1bdc6d83c970374d9d9de0a1d2 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 30 Jun 2020 10:24:55 -0700 Subject: [PATCH 36/49] Fabric: Implementation of `LayoutMetrics::overflowInset` Summary: `Overflow inset` is a field in LayoutMetrics that describes the external bounding box that covers area formed by all descendants of the view (that might stick out of the frame of the view). This information can be used for various optimizations across all rendering pipeline. Here is some of them: Improving hit-testing performance. Implementing a robust "remove invisible native views" feature. Improving `clipToBounds` performance. Improving View Flattening versatility. Most importantly, we need this for improving performance (and memory footprint) which we have to do to match parity with Paper's `removeClippedSubviews` feature. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22297325 fbshipit-source-id: cb238f0505d11ccabbe9db26f36a98b3172c70db --- .../view/yoga/YogaLayoutableShadowNode.cpp | 30 +++++++++++++++++++ .../fabric/core/layout/LayoutMetrics.h | 1 + .../fabric/core/layout/LayoutableShadowNode.h | 1 - 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 4094b390a2..3a876dba9c 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -352,10 +352,26 @@ void YogaLayoutableShadowNode::layoutTree( layout(layoutContext); } +static EdgeInsets calculateOverflowInset( + Rect containerFrame, + Rect contentFrame) { + auto size = containerFrame.size; + auto overflowInset = EdgeInsets{}; + overflowInset.left = std::min(contentFrame.getMinX(), Float{0.0}); + overflowInset.top = std::min(contentFrame.getMinY(), Float{0.0}); + overflowInset.right = + -std::max(contentFrame.getMaxX() - size.width, Float{0.0}); + overflowInset.bottom = + -std::max(contentFrame.getMaxY() - size.height, Float{0.0}); + return overflowInset; +} + void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { // Reading data from a dirtied node does not make sense. assert(!yogaNode_.isDirty()); + auto contentFrame = Rect{}; + for (auto childYogaNode : yogaNode_.getChildren()) { auto &childNode = *static_cast(childYogaNode->getContext()); @@ -391,6 +407,20 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { childNode.layout(layoutContext); } } + + auto layoutMetricsWithOverflowInset = childNode.getLayoutMetrics(); + if (layoutMetricsWithOverflowInset.displayType != DisplayType::None) { + contentFrame.unionInPlace(insetBy( + layoutMetricsWithOverflowInset.frame, + layoutMetricsWithOverflowInset.overflowInset)); + } + } + + if (yogaNode_.getStyle().overflow() == YGOverflowVisible) { + layoutMetrics_.overflowInset = + calculateOverflowInset(layoutMetrics_.frame, contentFrame); + } else { + layoutMetrics_.overflowInset = {}; } } diff --git a/ReactCommon/fabric/core/layout/LayoutMetrics.h b/ReactCommon/fabric/core/layout/LayoutMetrics.h index 014ce93c6c..d4708a899f 100644 --- a/ReactCommon/fabric/core/layout/LayoutMetrics.h +++ b/ReactCommon/fabric/core/layout/LayoutMetrics.h @@ -23,6 +23,7 @@ struct LayoutMetrics { DisplayType displayType{DisplayType::Flex}; LayoutDirection layoutDirection{LayoutDirection::Undefined}; Float pointScaleFactor{1.0}; + EdgeInsets overflowInset{}; Rect getContentFrame() const { return Rect{ diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h index 5882b3a78c..c4f45bf082 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h @@ -144,7 +144,6 @@ class LayoutableShadowNode : public ShadowNode { ShadowNode::Shared node, Point point); - protected: /* * Clean or Dirty layout state: * Indicates whether all nodes (and possibly their subtrees) along the path From 72a85040cdb8a2a15f0a536c52e00533191596eb Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 30 Jun 2020 10:24:55 -0700 Subject: [PATCH 37/49] Fabric: Unit test for Overflow Inset feature Summary: Subject. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D22297335 fbshipit-source-id: 299998094017f3396be2531f8d6bcfb787e94eed --- .../components/view/tests/LayoutTest.cpp | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 ReactCommon/fabric/components/view/tests/LayoutTest.cpp diff --git a/ReactCommon/fabric/components/view/tests/LayoutTest.cpp b/ReactCommon/fabric/components/view/tests/LayoutTest.cpp new file mode 100644 index 0000000000..f309557e5d --- /dev/null +++ b/ReactCommon/fabric/components/view/tests/LayoutTest.cpp @@ -0,0 +1,186 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +#include +#include +#include +#include +#include +#include + +namespace facebook { +namespace react { + +// *******************************************************┌─ABCD:────┐**** +// *******************************************************│ {70,-50} │**** +// *******************************************************│ {30,60} │**** +// *******************************************************│ │**** +// *******************************************************│ │**** +// *******************┌─A: {0,0}{50,50}──┐****************│ │**** +// *******************│ │****************│ │**** +// *******************│ ┌─AB:──────┐ │****************│ │**** +// *******************│ │ {10,10}{30,90}****************│ │**** +// *******************│ │ ┌─ABC: {10,10}{110,20}──────┤ ├───┐ +// *******************│ │ │ │ │ │ +// *******************│ │ │ └──────────┘ │ +// *******************│ │ └──────┬───┬───────────────────────────────┘ +// *******************│ │ │ │******************************** +// *******************└───┤ ├───┘******************************** +// ***********************│ │************************************ +// ***********************│ │************************************ +// ┌─ABE: {-60,50}{70,20}─┴───┐ │************************************ +// │ │ │************************************ +// │ │ │************************************ +// │ │ │************************************ +// │ │ │************************************ +// └──────────────────────┬───┘ │************************************ +// ***********************│ │************************************ +// ***********************└──────────┘************************************ + +class LayoutTest : public ::testing::Test { + protected: + ComponentBuilder builder_; + std::shared_ptr rootShadowNode_; + std::shared_ptr viewShadowNodeA_; + std::shared_ptr viewShadowNodeAB_; + std::shared_ptr viewShadowNodeABC_; + std::shared_ptr viewShadowNodeABCD_; + std::shared_ptr viewShadowNodeABE_; + + LayoutTest() : builder_(simpleComponentBuilder()) {} + + void initialize(bool enforceClippingForABC) { + // clang-format off + auto element = + Element() + .reference(rootShadowNode_) + .tag(1) + .props([] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + props.layoutConstraints = LayoutConstraints{{0,0}, {500, 500}}; + auto &yogaStyle = props.yogaStyle; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{200, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{200, YGUnitPoint}; + return sharedProps; + }) + .children({ + Element() + .reference(viewShadowNodeA_) + .props([] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + auto &yogaStyle = props.yogaStyle; + yogaStyle.positionType() = YGPositionTypeAbsolute; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{50, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{50, YGUnitPoint}; + return sharedProps; + }) + .children({ + Element() + .reference(viewShadowNodeAB_) + .props([] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + auto &yogaStyle = props.yogaStyle; + yogaStyle.positionType() = YGPositionTypeAbsolute; + yogaStyle.position()[YGEdgeLeft] = YGValue{10, YGUnitPoint}; + yogaStyle.position()[YGEdgeTop] = YGValue{10, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{30, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{90, YGUnitPoint}; + return sharedProps; + }) + .children({ + Element() + .reference(viewShadowNodeABC_) + .props([=] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + auto &yogaStyle = props.yogaStyle; + + if (enforceClippingForABC) { + yogaStyle.overflow() = YGOverflowHidden; + } + + yogaStyle.positionType() = YGPositionTypeAbsolute; + yogaStyle.position()[YGEdgeLeft] = YGValue{10, YGUnitPoint}; + yogaStyle.position()[YGEdgeTop] = YGValue{10, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{110, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{20, YGUnitPoint}; + return sharedProps; + }) + .children({ + Element() + .reference(viewShadowNodeABCD_) + .props([] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + auto &yogaStyle = props.yogaStyle; + yogaStyle.positionType() = YGPositionTypeAbsolute; + yogaStyle.position()[YGEdgeLeft] = YGValue{70, YGUnitPoint}; + yogaStyle.position()[YGEdgeTop] = YGValue{-50, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{30, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{60, YGUnitPoint}; + return sharedProps; + }) + }), + Element() + .reference(viewShadowNodeABE_) + .props([] { + auto sharedProps = std::make_shared(); + auto &props = *sharedProps; + auto &yogaStyle = props.yogaStyle; + yogaStyle.positionType() = YGPositionTypeAbsolute; + yogaStyle.position()[YGEdgeLeft] = YGValue{-60, YGUnitPoint}; + yogaStyle.position()[YGEdgeTop] = YGValue{50, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionWidth] = YGValue{70, YGUnitPoint}; + yogaStyle.dimensions()[YGDimensionHeight] = YGValue{20, YGUnitPoint}; + return sharedProps; + }) + }) + }) + }); + // clang-format on + + builder_.build(element); + + rootShadowNode_->layoutIfNeeded(); + } +}; + +TEST_F(LayoutTest, overflowInsetTest) { + initialize(false); + + auto layoutMetrics = viewShadowNodeA_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetrics.frame.size.width, 50); + EXPECT_EQ(layoutMetrics.frame.size.height, 50); + + EXPECT_EQ(layoutMetrics.overflowInset.left, -50); + EXPECT_EQ(layoutMetrics.overflowInset.top, -30); + EXPECT_EQ(layoutMetrics.overflowInset.right, -80); + EXPECT_EQ(layoutMetrics.overflowInset.bottom, -50); +} + +TEST_F(LayoutTest, overflowInsetWithClippingTest) { + initialize(true); + + auto layoutMetrics = viewShadowNodeA_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetrics.frame.size.width, 50); + EXPECT_EQ(layoutMetrics.frame.size.height, 50); + + EXPECT_EQ(layoutMetrics.overflowInset.left, -50); + EXPECT_EQ(layoutMetrics.overflowInset.top, 0); + EXPECT_EQ(layoutMetrics.overflowInset.right, -80); + EXPECT_EQ(layoutMetrics.overflowInset.bottom, -50); +} + +} // namespace react +} // namespace facebook From 753fdebcd4e03f4b809051ddbb23fd09413bbf35 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 30 Jun 2020 10:24:55 -0700 Subject: [PATCH 38/49] Fabric: Removing `get/setHasNewLayout` from LayoutableShadowNode interface Summary: It's a Yoga specific feature that does not need to exposed as LayoutableShadowNode. Removing this we save many virtual calls and add simplicity. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22304717 fbshipit-source-id: f2dda8309dd86b78cd2775971721d29e5317ffd5 --- .../view/yoga/YogaLayoutableShadowNode.cpp | 12 ++---------- .../components/view/yoga/YogaLayoutableShadowNode.h | 3 --- .../fabric/core/layout/LayoutableShadowNode.cpp | 5 ----- .../fabric/core/layout/LayoutableShadowNode.h | 7 ------- 4 files changed, 2 insertions(+), 25 deletions(-) diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 3a876dba9c..f17a8e41be 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -104,14 +104,6 @@ bool YogaLayoutableShadowNode::getIsLayoutClean() const { return !yogaNode_.isDirty(); } -bool YogaLayoutableShadowNode::getHasNewLayout() const { - return yogaNode_.getHasNewLayout(); -} - -void YogaLayoutableShadowNode::setHasNewLayout(bool hasNewLayout) { - yogaNode_.setHasNewLayout(hasNewLayout); -} - #pragma mark - Mutating Methods void YogaLayoutableShadowNode::enableMeasurement() { @@ -342,11 +334,11 @@ void YogaLayoutableShadowNode::layoutTree( &yogaNode_, YGUndefined, YGUndefined, YGDirectionInherit); } - if (getHasNewLayout()) { + if (yogaNode_.getHasNewLayout()) { auto layoutMetrics = layoutMetricsFromYogaNode(yogaNode_); layoutMetrics.pointScaleFactor = layoutContext.pointScaleFactor; setLayoutMetrics(layoutMetrics); - setHasNewLayout(false); + yogaNode_.setHasNewLayout(false); } layout(layoutContext); diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index 5372e8274c..5566e10590 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -73,9 +73,6 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { void dirtyLayout() override; bool getIsLayoutClean() const override; - void setHasNewLayout(bool hasNewLayout) override; - bool getHasNewLayout() const override; - /* * Computes layout using Yoga layout engine. * See `LayoutableShadowNode` for more details. diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp index b7f0458382..ba26ba40e3 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp @@ -238,11 +238,6 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint( SharedDebugStringConvertibleList LayoutableShadowNode::getDebugProps() const { auto list = SharedDebugStringConvertibleList{}; - if (getHasNewLayout()) { - list.push_back( - std::make_shared("hasNewLayout")); - } - if (!getIsLayoutClean()) { list.push_back(std::make_shared("dirty")); } diff --git a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h index c4f45bf082..1bcd163ead 100644 --- a/ReactCommon/fabric/core/layout/LayoutableShadowNode.h +++ b/ReactCommon/fabric/core/layout/LayoutableShadowNode.h @@ -153,13 +153,6 @@ class LayoutableShadowNode : public ShadowNode { virtual void dirtyLayout() = 0; virtual bool getIsLayoutClean() const = 0; - /* - * Indicates does the shadow node (or any descendand node of the node) - * get a new layout metrics during a previous layout pass. - */ - virtual void setHasNewLayout(bool hasNewLayout) = 0; - virtual bool getHasNewLayout() const = 0; - /* * Unifed methods to access text layout metrics. */ From 188a66daab59a966cd87a5dcfbf13952b84e50f5 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 30 Jun 2020 10:32:38 -0700 Subject: [PATCH 39/49] Fabric: Empty (no-op) `layoutSubviews` method was removed from RCTComponentView. Summary: The method does not do anything besides calling a super method. Even if this method does nothing special, overriding it can have negative performance implications. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D22309895 fbshipit-source-id: bd8237d15df20017629223f278b1b6ac628b0cc7 --- .../ComponentViews/View/RCTViewComponentView.mm | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm index c6503a635e..dfec3a5993 100644 --- a/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm @@ -53,18 +53,6 @@ using namespace facebook::react; } } -- (void)layoutSubviews -{ - [super layoutSubviews]; - // Consider whether using `updateLayoutMetrics:oldLayoutMetrics` - // isn't more appropriate for your use case. `layoutSubviews` is called - // by UIKit while `updateLayoutMetrics:oldLayoutMetrics` is called - // by React Native Renderer within `CATransaction`. - // If you are calling `setFrame:` or other methods that cause - // `layoutSubviews` to be triggered, `_contentView`'s and `_borderLayout`'s - // frames might get out of sync with `self.bounds`. -} - - (BOOL)pointInside:(CGPoint)point withEvent:(UIEvent *)event { if (UIEdgeInsetsEqualToEdgeInsets(self.hitTestEdgeInsets, UIEdgeInsetsZero)) { From 9e85b7ad889900cd57cd2f82286aa8e034b0a32b Mon Sep 17 00:00:00 2001 From: Vojtech Novak Date: Tue, 30 Jun 2020 12:49:40 -0700 Subject: [PATCH 40/49] ScrollView, HorizontalScrollView: do not ignore `null` `contentOffset` prop (#28760) Summary: motivation: I was just checking out https://github.com/facebook/react-native/commit/30cc158a875a0414cf53d4d5155410eea5d5aeea and noticed that the commit, I believe, is missing logic for when `contentOffset` is actually `null`. That is, consider you render `ScrollView` with `contentOffset` { x: 0, y: 100 } and then change that to null / undefined. I'd expect the content offset to invalidate (set to 0 - hope that's the default). ## Changelog [Android] [Fixed] - ScrollView, HorizontalScrollView: do not ignore `null` `contentOffset` prop Pull Request resolved: https://github.com/facebook/react-native/pull/28760 Test Plan: Tested locally within RNTester
code

```js const Ex = () => { let _scrollView: ?React.ElementRef = React.useRef(null); const [offset, setOffset] = React.useState({x: 0, y: 20}); setTimeout(() => { setOffset(undefined); }, 2000); return ( { console.log('onScroll!'); }} contentOffset={offset} scrollEventThrottle={200} style={styles.scrollView}> {ITEMS.map(createItemRow)}

Reviewed By: shergin Differential Revision: D22298676 Pulled By: JoshuaGross fbshipit-source-id: e411ba4c8a276908e354d59085d164a38ae253c0 --- .../react/views/scroll/ReactHorizontalScrollViewManager.java | 2 ++ .../com/facebook/react/views/scroll/ReactScrollViewManager.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java index 7f11c77abf..8b7215532a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollViewManager.java @@ -307,6 +307,8 @@ public class ReactHorizontalScrollViewManager extends ViewGroupManager double x = value.hasKey("x") ? value.getDouble("x") : 0; double y = value.hasKey("y") ? value.getDouble("y") : 0; view.reactScrollTo((int) PixelUtil.toPixelFromDIP(x), (int) PixelUtil.toPixelFromDIP(y)); + } else { + view.reactScrollTo(0, 0); } } From ab1f54c61cb4ac330d3cfd90ec1a52980eeb9e2e Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 30 Jun 2020 15:54:36 -0700 Subject: [PATCH 41/49] Do not log warning if JS responder isn't set in Fabric Summary: Changelog: [Internal] Do not log warning for "Invalid view set to be the JS responder". {F242140509} Fabric doesn't store views in view registry, therefore this warning would be shown to the developer everytime `[RCTUIManager setJSResponder]` is called. Reviewed By: shergin Differential Revision: D22309447 fbshipit-source-id: cac8985cdc79ab2d03a246cc2d9377472a64a683 --- React/Modules/RCTUIManager.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index 26573fe3f4..2079bbb7f6 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -1444,7 +1444,8 @@ RCT_EXPORT_METHOD(setJSResponder { [self addUIBlock:^(__unused RCTUIManager *uiManager, NSDictionary *viewRegistry) { _jsResponder = viewRegistry[reactTag]; - if (!_jsResponder) { + // Fabric view's are not stored in viewRegistry. We avoid logging a warning in that case. + if (!_jsResponder && !RCTUIManagerTypeForTagIsFabric(reactTag)) { RCTLogWarn(@"Invalid view set to be the JS responder - tag %@", reactTag); } }]; From 85f1e984bb8d6decee4a771dc79055a52a5ec52b Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 30 Jun 2020 16:09:03 -0700 Subject: [PATCH 42/49] Remove references to enableFabricStopAllSurfacesOnTeardown Summary: According to our experiments it's not better than "stop surface on unmount" in any way, and might regress some metrics. Unclear why, but if it's not necessary and doesn't seem to help, it doesn't make sense to continue this experiment. We still have a mechanism on the C++ side to stop outstanding surfaces on teardown that does the same thing. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22318864 fbshipit-source-id: 7e678c63e4884382e57d996a7f4c4b7b24c8193a --- .../com/facebook/react/config/ReactFeatureFlags.java | 3 --- .../java/com/facebook/react/fabric/FabricUIManager.java | 9 --------- 2 files changed, 12 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index ce0b3d4dba..e0882c0cbc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -81,9 +81,6 @@ public class ReactFeatureFlags { /** Feature flag to configure initialization of Fabric surfaces. */ public static boolean enableFabricStartSurfaceWithLayoutMetrics = true; - /** Feature flag to have FabricUIManager teardown stop all active surfaces. */ - public static boolean enableFabricStopAllSurfacesOnTeardown = false; - /** Feature flag to use stopSurface when ReactRootView is unmounted. */ public static boolean enableStopSurfaceOnRootViewUnmount = false; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index e76a5746e0..1edbe4229c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -297,15 +297,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { // memory immediately. mDispatchUIFrameCallback.stop(); - // Stop all attached surfaces - if (ReactFeatureFlags.enableFabricStopAllSurfacesOnTeardown) { - FLog.e(TAG, "stop all attached surfaces"); - for (int surfaceId : mReactContextForRootTag.keySet()) { - FLog.e(TAG, "stop attached surface: " + surfaceId); - stopSurface(surfaceId); - } - } - mBinding.unregister(); mBinding = null; From f493a316fda963b1cb33aaaf7fdf6182b98d4ff8 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 30 Jun 2020 16:09:03 -0700 Subject: [PATCH 43/49] Remove flags around Catalyst teardown Summary: These flags haven't been used in months. They were useful to uncover some race conditions, but will not be iterated further. The Venice project will obviate the concerns that sparked these experiments in the first place. These flags have been hardcoded to false for a while. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22319204 fbshipit-source-id: 09415f3bb1ca56e15f357210e966d0483ff384f2 --- .../react/bridge/CatalystInstanceImpl.java | 105 ------------------ .../facebook/react/bridge/ReactContext.java | 3 - .../react/config/ReactFeatureFlags.java | 17 +-- 3 files changed, 1 insertion(+), 124 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index e225306e68..dae969346a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -340,16 +340,6 @@ public class CatalystInstanceImpl implements CatalystInstance { FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() start"); UiThreadUtil.assertOnUiThread(); - if (ReactFeatureFlags.useCatalystTeardownV2) { - destroyV2(); - } else { - destroyV1(); - } - } - - @ThreadConfined(UI) - public void destroyV1() { - FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroyV1() start"); UiThreadUtil.assertOnUiThread(); if (mDestroyed) { @@ -427,101 +417,6 @@ public class CatalystInstanceImpl implements CatalystInstance { Systrace.unregisterListener(mTraceListener); } - /** - * Destroys this catalyst instance, waiting for any other threads in ReactQueueConfiguration - * (besides the UI thread) to finish running. Must be called from the UI thread so that we can - * fully shut down other threads. - */ - @ThreadConfined(UI) - public void destroyV2() { - FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroyV2() start"); - UiThreadUtil.assertOnUiThread(); - - if (mDestroyed) { - return; - } - - // TODO: tell all APIs to shut down - ReactMarker.logMarker(ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_START); - mDestroyed = true; - mNativeModulesThreadDestructionComplete = false; - mJSThreadDestructionComplete = false; - - mNativeModulesQueueThread.runOnQueue( - new Runnable() { - @Override - public void run() { - FLog.d("CatalystInstanceImpl", ".destroy on native modules thread"); - mNativeModuleRegistry.notifyJSInstanceDestroy(); - - // Notifies all JSI modules that they are being destroyed, including the FabricUIManager - // and Fabric Scheduler - mJSIModuleRegistry.notifyJSInstanceDestroy(); - boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0); - if (!mBridgeIdleListeners.isEmpty()) { - for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) { - if (!wasIdle) { - listener.onTransitionToBridgeIdle(); - } - listener.onBridgeDestroyed(); - } - } - - mNativeModulesThreadDestructionComplete = true; - FLog.d("CatalystInstanceImpl", ".destroy on native modules thread finished"); - } - }); - - getReactQueueConfiguration() - .getJSQueueThread() - .runOnQueue( - new Runnable() { - @Override - public void run() { - FLog.d("CatalystInstanceImpl", ".destroy on JS thread"); - // We need to destroy the TurboModuleManager on the JS Thread - if (mTurboModuleManagerJSIModule != null) { - mTurboModuleManagerJSIModule.onCatalystInstanceDestroy(); - } - - mJSThreadDestructionComplete = true; - FLog.d("CatalystInstanceImpl", ".destroy on JS thread finished"); - } - }); - - // Wait until destruction is complete - long waitStartTime = System.currentTimeMillis(); - while (!mNativeModulesThreadDestructionComplete || !mJSThreadDestructionComplete) { - // Never wait here, blocking the UI thread, for more than 100ms - if ((System.currentTimeMillis() - waitStartTime) > 100) { - FLog.w( - ReactConstants.TAG, - "CatalystInstanceImpl.destroy() timed out waiting for Native Modules and JS thread teardown"); - break; - } - } - - // Kill non-UI threads from neutral third party - // potentially expensive, so don't run on UI thread - - // contextHolder is used as a lock to guard against - // other users of the JS VM having the VM destroyed - // underneath them, so notify them before we reset - // Native - mJavaScriptContextHolder.clear(); - - // Imperatively destruct the C++ CatalystInstance rather than - // wait for the JVM's GC to free it. - mHybridData.resetNative(); - - getReactQueueConfiguration().destroy(); - FLog.d(ReactConstants.TAG, "CatalystInstanceImpl.destroy() end"); - ReactMarker.logMarker(ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_END); - - // This is a noop if the listener was not yet registered. - Systrace.unregisterListener(mTraceListener); - } - @Override public boolean isDestroyed() { return mDestroyed; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 126237e74e..27d4487017 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -296,9 +296,6 @@ public class ReactContext extends ContextWrapper { mDestroyed = true; if (mCatalystInstance != null) { mCatalystInstance.destroy(); - if (ReactFeatureFlags.nullifyCatalystInstanceOnDestroy) { - mCatalystInstance = null; - } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index e0882c0cbc..94e6349153 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -41,22 +41,7 @@ public class ReactFeatureFlags { * inside view manager will be called instead. */ public static boolean useViewManagerDelegatesForCommands = false; - - /** - * Should this application use Catalyst Teardown V2? This is an experiment to use a V2 of the - * CatalystInstanceImpl `destroy` method. - */ - public static boolean useCatalystTeardownV2 = false; - - /** - * When the ReactContext is destroyed, should the CatalystInstance immediately be nullified? This - * is the safest thing to do since the CatalystInstance shouldn't be used, and should be - * garbage-collected after it's destroyed, but this is a breaking change in that many native - * modules assume that a ReactContext will always have a CatalystInstance. This will be deleted - * and the CatalystInstance will always be destroyed in some future release. - */ - public static boolean nullifyCatalystInstanceOnDestroy = false; - + /** * This react flag enables a custom algorithm for the getChildVisibleRect() method in the classes * ReactViewGroup, ReactHorizontalScrollView and ReactScrollView. From 09f360b13d8b97141f662941a4482a267d8261be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= Date: Tue, 30 Jun 2020 16:27:40 -0700 Subject: [PATCH 44/49] Codegen: Move properties functions out of methods file Summary: Moving property handling functions to their own properties.js file. No changes other than adding types to params. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22208243 fbshipit-source-id: 4a7d2c6e19c151954da793d399af9a256a4a40b7 --- .../src/parsers/flow/modules/methods.js | 135 +--------------- .../src/parsers/flow/modules/properties.js | 149 ++++++++++++++++++ 2 files changed, 155 insertions(+), 129 deletions(-) create mode 100644 packages/react-native-codegen/src/parsers/flow/modules/properties.js diff --git a/packages/react-native-codegen/src/parsers/flow/modules/methods.js b/packages/react-native-codegen/src/parsers/flow/modules/methods.js index e6e34b6af2..37bc10758d 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/methods.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/methods.js @@ -14,144 +14,21 @@ import type { NativeModuleMethodTypeShape, FunctionTypeAnnotationParam, FunctionTypeAnnotationReturn, - FunctionTypeAnnotationParamTypeAnnotation, - ObjectParamTypeAnnotation, } from '../../../CodegenSchema.js'; -import type {TypeMap} from '../utils.js'; +import type {ASTNode, TypeMap} from '../utils.js'; const {getValueFromTypes} = require('../utils.js'); +const { + getElementTypeForArrayOrObject, + getObjectProperties, +} = require('./properties'); // $FlowFixMe there's no flowtype for ASTs type MethodAST = Object; -function getObjectProperties( - name: string, - objectParam, - paramName: string, - types: TypeMap, -): $ReadOnlyArray { - return objectParam.properties.map(objectTypeProperty => { - let optional = objectTypeProperty.optional; - let value = objectTypeProperty.value; - if (value.type === 'NullableTypeAnnotation') { - if ( - objectTypeProperty.value.typeAnnotation.type !== 'StringTypeAnnotation' - ) { - optional = true; - } - value = objectTypeProperty.value.typeAnnotation; - } - return { - optional, - name: objectTypeProperty.key.name, - typeAnnotation: getElementTypeForArrayOrObject( - name, - value, - paramName, - types, - ), - }; - }); -} - -function getElementTypeForArrayOrObject( - name, - arrayParam, - paramName, - types: TypeMap, -): FunctionTypeAnnotationParamTypeAnnotation | typeof undefined { - const typeAnnotation = getValueFromTypes(arrayParam, types); - const type = - typeAnnotation.type === 'GenericTypeAnnotation' - ? typeAnnotation.id.name - : typeAnnotation.type; - - switch (type) { - case 'RootTag': - return { - type: 'ReservedFunctionValueTypeAnnotation', - name: 'RootTag', - }; - case 'Array': - case '$ReadOnlyArray': - if ( - typeAnnotation.typeParameters && - typeAnnotation.typeParameters.params[0] - ) { - return { - type: 'ArrayTypeAnnotation', - elementType: getElementTypeForArrayOrObject( - name, - typeAnnotation.typeParameters.params[0], - 'returning value', - types, - ), - }; - } else { - throw new Error( - `Unsupported type for ${name}, param: "${paramName}": expected to find annotation for type of nested array contents`, - ); - } - case 'ObjectTypeAnnotation': - return { - type: 'ObjectTypeAnnotation', - properties: getObjectProperties(name, typeAnnotation, paramName, types), - }; - case '$ReadOnly': - if ( - typeAnnotation.typeParameters.params && - typeAnnotation.typeParameters.params[0] - ) { - return { - type: 'ObjectTypeAnnotation', - properties: getObjectProperties( - name, - typeAnnotation.typeParameters.params[0], - paramName, - types, - ), - }; - } else { - throw new Error( - `Unsupported param for method "${name}", param "${paramName}". No type specified for $ReadOnly`, - ); - } - case 'AnyTypeAnnotation': - return { - type, - }; - case 'NumberTypeAnnotation': - case 'BooleanTypeAnnotation': - return { - type, - }; - case 'StringTypeAnnotation': - case 'Stringish': - return { - type: 'StringTypeAnnotation', - }; - case 'Int32': - return { - type: 'Int32TypeAnnotation', - }; - case 'Float': - return { - type: 'FloatTypeAnnotation', - }; - case 'TupleTypeAnnotation': - case 'UnionTypeAnnotation': - return undefined; - default: - // TODO T67565166: Generic objects are not type safe and should be disallowed in the schema. - return { - type: 'GenericObjectTypeAnnotation', - }; - } -} - function getTypeAnnotationForParam( name: string, - paramAnnotation, + paramAnnotation: ASTNode, types: TypeMap, ): FunctionTypeAnnotationParam { let param = paramAnnotation; diff --git a/packages/react-native-codegen/src/parsers/flow/modules/properties.js b/packages/react-native-codegen/src/parsers/flow/modules/properties.js new file mode 100644 index 0000000000..bd7b83a7dc --- /dev/null +++ b/packages/react-native-codegen/src/parsers/flow/modules/properties.js @@ -0,0 +1,149 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + */ + +'use strict'; + +import type { + FunctionTypeAnnotationParamTypeAnnotation, + ObjectParamTypeAnnotation, +} from '../../../CodegenSchema.js'; + +import type {ASTNode, TypeMap} from '../utils.js'; +const {getValueFromTypes} = require('../utils.js'); + +function getObjectProperties( + name: string, + objectParam: ASTNode, + paramName: string, + types: TypeMap, +): $ReadOnlyArray { + return objectParam.properties.map(objectTypeProperty => { + let optional = objectTypeProperty.optional; + let value = objectTypeProperty.value; + if (value.type === 'NullableTypeAnnotation') { + if ( + objectTypeProperty.value.typeAnnotation.type !== 'StringTypeAnnotation' + ) { + optional = true; + } + value = objectTypeProperty.value.typeAnnotation; + } + return { + optional, + name: objectTypeProperty.key.name, + typeAnnotation: getElementTypeForArrayOrObject( + name, + value, + paramName, + types, + ), + }; + }); +} + +function getElementTypeForArrayOrObject( + name: string, + arrayParam: ASTNode, + paramName: string, + types: TypeMap, +): FunctionTypeAnnotationParamTypeAnnotation | typeof undefined { + const typeAnnotation = getValueFromTypes(arrayParam, types); + const type = + typeAnnotation.type === 'GenericTypeAnnotation' + ? typeAnnotation.id.name + : typeAnnotation.type; + + switch (type) { + case 'RootTag': + return { + type: 'ReservedFunctionValueTypeAnnotation', + name: 'RootTag', + }; + case 'Array': + case '$ReadOnlyArray': + if ( + typeAnnotation.typeParameters && + typeAnnotation.typeParameters.params[0] + ) { + return { + type: 'ArrayTypeAnnotation', + elementType: getElementTypeForArrayOrObject( + name, + typeAnnotation.typeParameters.params[0], + 'returning value', + types, + ), + }; + } else { + throw new Error( + `Unsupported type for ${name}, param: "${paramName}": expected to find annotation for type of nested array contents`, + ); + } + case 'ObjectTypeAnnotation': + return { + type: 'ObjectTypeAnnotation', + properties: getObjectProperties(name, typeAnnotation, paramName, types), + }; + case '$ReadOnly': + if ( + typeAnnotation.typeParameters.params && + typeAnnotation.typeParameters.params[0] + ) { + return { + type: 'ObjectTypeAnnotation', + properties: getObjectProperties( + name, + typeAnnotation.typeParameters.params[0], + paramName, + types, + ), + }; + } else { + throw new Error( + `Unsupported param for method "${name}", param "${paramName}". No type specified for $ReadOnly`, + ); + } + case 'AnyTypeAnnotation': + return { + type, + }; + case 'NumberTypeAnnotation': + case 'BooleanTypeAnnotation': + return { + type, + }; + case 'StringTypeAnnotation': + case 'Stringish': + return { + type: 'StringTypeAnnotation', + }; + case 'Int32': + return { + type: 'Int32TypeAnnotation', + }; + case 'Float': + return { + type: 'FloatTypeAnnotation', + }; + case 'TupleTypeAnnotation': + case 'UnionTypeAnnotation': + return undefined; + default: + // TODO T67565166: Generic objects are not type safe and should be disallowed in the schema. + return { + type: 'GenericObjectTypeAnnotation', + }; + } +} + +module.exports = { + getElementTypeForArrayOrObject, + getObjectProperties, +}; From c2d827f160c3249af560f83956bdea487b3366d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= Date: Tue, 30 Jun 2020 19:05:16 -0700 Subject: [PATCH 45/49] Codegen: Allow use of arbitrary Yarn binary in codegen script Summary: Splits the codegen script into functions, and makes use of $YARN_BINARY for portability (defaults to `yarn` if not provided). Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22148360 fbshipit-source-id: 9e86b3e0f7f77bf3a635bf6be204170333dd5e65 --- scripts/generate-native-modules-specs.sh | 45 ++++++++++++++---------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/scripts/generate-native-modules-specs.sh b/scripts/generate-native-modules-specs.sh index 6f77c36c50..fc567f9f38 100755 --- a/scripts/generate-native-modules-specs.sh +++ b/scripts/generate-native-modules-specs.sh @@ -20,28 +20,37 @@ set -e +THIS_DIR=$(cd -P "$(dirname "$(readlink "${BASH_SOURCE[0]}" || echo "${BASH_SOURCE[0]}")")" && pwd) +RN_DIR=$(cd "$THIS_DIR/.." && pwd) +CODEGEN_DIR=$(cd "$RN_DIR/packages/react-native-codegen" && pwd) +OUTPUT_DIR="${1:-$RN_DIR/Libraries/FBReactNativeSpec/FBReactNativeSpec}" +SCHEMA_FILE="$RN_DIR/schema-native-modules.json" +YARN_BINARY="${YARN_BINARY:-yarn}" + describe () { printf "\\n\\n>>>>> %s\\n\\n\\n" "$1" } -THIS_DIR=$(cd -P "$(dirname "$(readlink "${BASH_SOURCE[0]}" || echo "${BASH_SOURCE[0]}")")" && pwd) -RN_DIR=$(cd "$THIS_DIR/.." && pwd) +step_build_codegen () { + describe "Building react-native-codegen package" + pushd "$CODEGEN_DIR" >/dev/null || exit + "$YARN_BINARY" + "$YARN_BINARY" build + popd >/dev/null || exit +} -SRCS_DIR=$(cd "$RN_DIR/Libraries" && pwd) -SCHEMA_FILE="$RN_DIR/schema-native-modules.json" +step_gen_schema () { + describe "Generating schema from flow types" + SRCS_DIR=$(cd "$RN_DIR/Libraries" && pwd) + grep --exclude NativeSampleTurboModule.js --exclude NativeUIManager.js --include=Native\*.js -rnwl "$SRCS_DIR" -e 'export interface Spec extends TurboModule' -e "export default \(TurboModuleRegistry.get(Enforcing)?\('.*\): Spec\);/" \ + | xargs "$YARN_BINARY" node "$CODEGEN_DIR/lib/cli/combine/combine-js-to-schema-cli.js" "$SCHEMA_FILE" +} -if [ -n "$1" ]; then - OUTPUT_DIR="$1" -fi +step_gen_specs () { + describe "Generating native code from schema" + "$YARN_BINARY" --silent node scripts/generate-native-modules-specs-cli.js "$SCHEMA_FILE" "$OUTPUT_DIR" +} -pushd "$RN_DIR/packages/react-native-codegen" >/dev/null - yarn - yarn build -popd >/dev/null - -describe "Generating schema from flow types" -grep --exclude NativeUIManager.js --include=Native\*.js -rnwl "$SRCS_DIR" -e 'export interface Spec extends TurboModule' -e "export default \(TurboModuleRegistry.get(Enforcing)?\('.*\): Spec\);/" \ - | xargs yarn node packages/react-native-codegen/lib/cli/combine/combine-js-to-schema-cli.js "$SCHEMA_FILE" - -describe "Generating native code from schema" -yarn node scripts/generate-native-modules-specs-cli.js "$SCHEMA_FILE" "$OUTPUT_DIR" +step_build_codegen +step_gen_schema +step_gen_specs From 3c3f8ca270028fc0b6c20eba4d95561298a9d26f Mon Sep 17 00:00:00 2001 From: Riley Dulin Date: Tue, 30 Jun 2020 21:13:53 -0700 Subject: [PATCH 46/49] Add a special case for assert in installConsoleFunction Summary: The Hermes inspector always logged the message for `console.assert`. It instead should check the first argument, and only if that argument is false should it log. Also remove the polyfill code that tried to avoid this situation. Changelog: [Internal] Reviewed By: mhorowitz Differential Revision: D22299186 fbshipit-source-id: cdf4f8957d4db3d171d6673a82c7fc32b7152af3 --- Libraries/polyfills/console.js | 10 +---- ReactCommon/hermes/inspector/Inspector.cpp | 52 +++++++++++++++++++--- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/Libraries/polyfills/console.js b/Libraries/polyfills/console.js index 962e52b5e9..f82fb4d05b 100644 --- a/Libraries/polyfills/console.js +++ b/Libraries/polyfills/console.js @@ -580,15 +580,7 @@ if (global.nativeLoggingHook) { const reactNativeMethod = console[methodName]; if (originalConsole[methodName]) { console[methodName] = function() { - // TODO(T43930203): remove this special case once originalConsole.assert properly checks - // the condition - if (methodName === 'assert') { - if (!arguments[0]) { - originalConsole.assert(...arguments); - } - } else { - originalConsole[methodName](...arguments); - } + originalConsole[methodName](...arguments); reactNativeMethod.apply(console, arguments); }; } diff --git a/ReactCommon/hermes/inspector/Inspector.cpp b/ReactCommon/hermes/inspector/Inspector.cpp index c2b5871211..14cb76cad3 100644 --- a/ReactCommon/hermes/inspector/Inspector.cpp +++ b/ReactCommon/hermes/inspector/Inspector.cpp @@ -136,6 +136,29 @@ Inspector::~Inspector() { debugger_.setEventObserver(nullptr); } +static bool toBoolean(jsi::Runtime &runtime, const jsi::Value &val) { + // Based on Operations.cpp:toBoolean in the Hermes VM. + if (val.isUndefined() || val.isNull()) { + return false; + } + if (val.isBool()) { + return val.getBool(); + } + if (val.isNumber()) { + double m = val.getNumber(); + return m != 0 && !std::isnan(m); + } + if (val.isSymbol() || val.isObject()) { + return true; + } + if (val.isString()) { + std::string s = val.getString(runtime).utf8(runtime); + return !s.empty(); + } + assert(false && "All cases should be covered"); + return false; +} + void Inspector::installConsoleFunction( jsi::Object &console, std::shared_ptr &originalConsole, @@ -169,11 +192,30 @@ void Inspector::installConsoleFunction( } if (auto inspector = weakInspector.lock()) { - jsi::Array argsArray(runtime, count); - for (size_t index = 0; index < count; ++index) - argsArray.setValueAtIndex(runtime, index, args[index]); - inspector->logMessage( - ConsoleMessageInfo{chromeType, std::move(argsArray)}); + if (name != "assert") { + // All cases other than assert just log a simple message. + jsi::Array argsArray(runtime, count); + for (size_t index = 0; index < count; ++index) + argsArray.setValueAtIndex(runtime, index, args[index]); + inspector->logMessage( + ConsoleMessageInfo{chromeType, std::move(argsArray)}); + return jsi::Value::undefined(); + } + // console.assert needs to check the first parameter before + // logging. + if (count == 0) { + // No parameters, throw a blank assertion failed message. + inspector->logMessage( + ConsoleMessageInfo{chromeType, jsi::Array(runtime, 0)}); + } else if (!toBoolean(runtime, args[0])) { + // Shift the message array down by one to not include the + // condition. + jsi::Array argsArray(runtime, count - 1); + for (size_t index = 1; index < count; ++index) + argsArray.setValueAtIndex(runtime, index, args[index]); + inspector->logMessage( + ConsoleMessageInfo{chromeType, std::move(argsArray)}); + } } return jsi::Value::undefined(); From 0aa8ed6361e981c509cb8370e2b404c08a3bc83d Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 30 Jun 2020 22:03:38 -0700 Subject: [PATCH 47/49] Remove useless Mounting diagnostic error Summary: While in theory we should never delete views before removing them from the hierarchy, there are some exceptions: (1) Some mysterious cases that don't seem like bugs, but where the child still seems to keep a reference to the parent: (2) When deleting views as part of stopSurface. On #1: in the past we had issues when we assumed that ViewManager.getChildCount() would return an accurate count. Sometimes it's just... wrong. Here, I've found at least one case where a View still has a parent after it's removed from the View hierarchy. I assume this is undocumented Android behavior or an Android bug, but either way, there's nothing I can do about it. On #2: there are valid cases where we want to delete a View without explicitly removing it from the View hierarchy (it will eventually be removed from the hierarchy when the Root view is unmounted, but it may still be "in" a View hierarchy when it's deleted). Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22321374 fbshipit-source-id: 9667bbe778c418f0216550638dc26ca48a58e5fa --- .../facebook/react/fabric/mounting/MountingManager.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 47d6582698..3c33450fc1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -455,15 +455,6 @@ public class MountingManager { View view = viewState.mView; if (view != null) { - ViewParent parentView = view.getParent(); - - if (parentView != null) { - ReactSoftException.logSoftException( - TAG, - new IllegalStateException( - "Warning: Deleting view that is still attached to parent: [" + reactTag + "]")); - } - dropView(view); } else { mTagToViewState.remove(reactTag); From 0cef464fd25e4856b18dd10c650267863feb12b5 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 30 Jun 2020 22:03:38 -0700 Subject: [PATCH 48/49] StopSurface optimizations Summary: (1) As soon as we know we can StopSurface, stop executing all mountitems by clearing out the root tag. (2) If a surface has been stopped and there's a batch of MountItems to execute against it, execute only the "delete" operations to clear views from memory. Both of these should reduce memory usage and improve speed slightly around navigation pops. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22321389 fbshipit-source-id: 96a8292a8442528f1bba50d35208885cc4168170 --- .../react/fabric/FabricUIManager.java | 3 +- .../mounting/mountitems/BatchMountItem.java | 38 +++++++++++++++---- .../RemoveDeleteMultiMountItem.java | 21 +++++++++- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 1edbe4229c..3f3eb338d1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -253,8 +253,8 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { @ThreadConfined(ANY) @Override public void stopSurface(int surfaceID) { - mBinding.stopSurface(surfaceID); mReactContextForRootTag.remove(surfaceID); + mBinding.stopSurface(surfaceID); } @Override @@ -853,6 +853,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { BatchMountItem batchMountItem = (BatchMountItem) mountItem; if (!surfaceActiveForExecution( batchMountItem.getRootTag(), "dispatchMountItems BatchMountItem")) { + batchMountItem.executeDeletes(mMountingManager); continue; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java index 91e736d533..7aebdd3d07 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/BatchMountItem.java @@ -45,21 +45,18 @@ public class BatchMountItem implements MountItem { mCommitNumber = commitNumber; } - @Override - public void execute(@NonNull MountingManager mountingManager) { + private void beginMarkers(String reason) { Systrace.beginSection( - Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "FabricUIManager::mountViews - " + mSize + " items"); + Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, + "FabricUIManager::" + reason + " - " + mSize + " items"); if (mCommitNumber > 0) { ReactMarker.logFabricMarker( ReactMarkerConstants.FABRIC_BATCH_EXECUTION_START, null, mCommitNumber); } + } - for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) { - MountItem mountItem = mMountItems[mountItemIndex]; - mountItem.execute(mountingManager); - } - + private void endMarkers() { if (mCommitNumber > 0) { ReactMarker.logFabricMarker( ReactMarkerConstants.FABRIC_BATCH_EXECUTION_END, null, mCommitNumber); @@ -68,6 +65,31 @@ public class BatchMountItem implements MountItem { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } + @Override + public void execute(@NonNull MountingManager mountingManager) { + beginMarkers("mountViews"); + + for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) { + MountItem mountItem = mMountItems[mountItemIndex]; + mountItem.execute(mountingManager); + } + + endMarkers(); + } + + public void executeDeletes(@NonNull MountingManager mountingManager) { + beginMarkers("deleteViews"); + + for (int mountItemIndex = 0; mountItemIndex < mSize; mountItemIndex++) { + MountItem mountItem = mMountItems[mountItemIndex]; + if (mountItem instanceof RemoveDeleteMultiMountItem) { + ((RemoveDeleteMultiMountItem) mountItem).executeDeletes(mountingManager, true); + } + } + + endMarkers(); + } + public int getRootTag() { return mRootTag; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java index daf3ed5ca7..8a25f84b74 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/RemoveDeleteMultiMountItem.java @@ -56,11 +56,30 @@ public class RemoveDeleteMultiMountItem implements MountItem { } // After removing all views, delete all views marked for deletion. + executeDeletes(mountingManager, false); + } + + /** + * Execute only deletion operations. When being executed as part of shutdown/stopping surface, + * deletion failures can be ignored. For example: if there is a batch of MountItems being executed + * as part of stopSurface, a "Create" that is not executed may have a matching "Delete". The + * Delete will fail but we can safely ignore it in those cases. + * + * @param mountingManager + * @param ignoreFailures + */ + public void executeDeletes(@NonNull MountingManager mountingManager, boolean ignoreFailures) { for (int i = 0; i < mMetadata.length; i += 4) { int flags = mMetadata[i + FLAGS_INDEX]; if ((flags & DELETE_FLAG) != 0) { int tag = mMetadata[i + TAG_INDEX]; - mountingManager.deleteView(tag); + try { + mountingManager.deleteView(tag); + } catch (IllegalStateException e) { + if (!ignoreFailures) { + throw e; + } + } } } } From e261f022fe6a7653b79330f878fed143725f5324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= Date: Tue, 30 Jun 2020 23:55:12 -0700 Subject: [PATCH 49/49] Codegen: List type aliases in modules schema Summary: The current parser behavior flattens out any object type aliases into ObjectTypeAnnotations. Generators can treat these as regular objects and generate the applicable native code. This, however, can lead to repetition whenever an object type alias is re-used in the same native module. I propose we treat these as a special case, using a TypeAliasTypeAnnotation to denote them as type aliases. Generators can look up the actual signature for a given object alias by referring to the "aliases" array that is provided in the schema. **Proposed schema change:** Adds an "aliases" key to each module in the schema: ``` export type NativeModuleShape = $ReadOnly<{| properties: $ReadOnlyArray, aliases: $ReadOnlyArray<{| name: string, typeAnnotation: | $ReadOnly<{| type: 'ObjectTypeAnnotation', properties: $ReadOnlyArray, |}> | $ReadOnly, |}>, |}>; ``` Example: ``` { modules: { SampleTurboModule: { nativeModules: { SampleTurboModule: { aliases: [], properties: [], }, }, }, }, } ``` Method parameters will now support the new 'TypeAliasTypeAnnotation' wherever a param might have used a 'ObjectTypeAnnotation': ``` export type TypeAliasTypeAnnotation = $ReadOnly<{| type: 'TypeAliasTypeAnnotation', name: string, |}>; ``` Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22200700 fbshipit-source-id: 15684620783c752f2fb482ba4b88d1fd1cc07540 --- .../react-native-codegen/src/CodegenSchema.js | 27 ++- .../generators/modules/GenerateModuleCpp.js | 5 +- .../src/generators/modules/GenerateModuleH.js | 1 + .../modules/GenerateModuleHObjCpp.js | 1 + .../modules/ObjCppUtils/GenerateStructs.js | 2 + .../GenerateStructsForConstants.js | 2 + .../modules/__test_fixtures__/fixtures.js | 7 + .../modules/__test_fixtures__/fixtures.js | 43 ++++ .../__snapshots__/module-parser-test.js.snap | 213 ++++++++++++++++-- .../src/parsers/flow/modules/aliases.js | 70 ++++++ .../src/parsers/flow/modules/index.js | 33 ++- .../src/parsers/flow/modules/methods.js | 10 + .../src/parsers/flow/modules/properties.js | 14 +- .../src/parsers/flow/modules/schema.js | 5 +- 14 files changed, 402 insertions(+), 31 deletions(-) create mode 100644 packages/react-native-codegen/src/parsers/flow/modules/aliases.js diff --git a/packages/react-native-codegen/src/CodegenSchema.js b/packages/react-native-codegen/src/CodegenSchema.js index 8706592132..5ae6d4a247 100644 --- a/packages/react-native-codegen/src/CodegenSchema.js +++ b/packages/react-native-codegen/src/CodegenSchema.js @@ -55,6 +55,11 @@ export type StringTypeAnnotation = $ReadOnly<{| type: 'StringTypeAnnotation', |}>; +export type TypeAliasTypeAnnotation = $ReadOnly<{| + type: 'TypeAliasTypeAnnotation', + name: string, +|}>; + export type EventObjectPropertyType = | $ReadOnly<{| type: 'BooleanTypeAnnotation', @@ -223,19 +228,25 @@ export type FunctionTypeAnnotationParamTypeAnnotation = |}> | $ReadOnly<{| type: 'ArrayTypeAnnotation', - elementType: ?FunctionTypeAnnotationParamTypeAnnotation, + elementType: + | ?FunctionTypeAnnotationParamTypeAnnotation + | ?TypeAliasTypeAnnotation, |}> | $ReadOnly<{| type: 'ObjectTypeAnnotation', properties: ?$ReadOnlyArray, |}>; -export type FunctionTypeAnnotationReturnArrayElementType = FunctionTypeAnnotationParamTypeAnnotation; +export type FunctionTypeAnnotationReturnArrayElementType = + | FunctionTypeAnnotationParamTypeAnnotation + | TypeAliasTypeAnnotation; export type ObjectParamTypeAnnotation = $ReadOnly<{| optional: boolean, name: string, - typeAnnotation?: FunctionTypeAnnotationParamTypeAnnotation, // TODO (T67898313): Workaround for NativeLinking's use of union type, typeAnnotations should not be optional + typeAnnotation?: + | FunctionTypeAnnotationParamTypeAnnotation + | TypeAliasTypeAnnotation, // TODO (T67898313): Workaround for NativeLinking's use of union type, typeAnnotations should not be optional |}>; export type FunctionTypeAnnotationReturn = @@ -265,7 +276,9 @@ export type FunctionTypeAnnotationReturn = export type FunctionTypeAnnotationParam = $ReadOnly<{| nullable: boolean, name: string, - typeAnnotation: FunctionTypeAnnotationParamTypeAnnotation, + typeAnnotation: + | FunctionTypeAnnotationParamTypeAnnotation + | TypeAliasTypeAnnotation, |}>; export type FunctionTypeAnnotation = $ReadOnly<{| @@ -280,7 +293,13 @@ export type NativeModuleMethodTypeShape = $ReadOnly<{| typeAnnotation: FunctionTypeAnnotation, |}>; +export type ObjectTypeAliasTypeShape = $ReadOnly<{| + type: 'ObjectTypeAnnotation', + properties: $ReadOnlyArray, +|}>; + export type NativeModuleShape = $ReadOnly<{| + aliases: $ReadOnly<{[aliasName: string]: ObjectTypeAliasTypeShape, ...}>, properties: $ReadOnlyArray, |}>; diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js index 5acd433a04..c87add8b58 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleCpp.js @@ -75,7 +75,7 @@ function traverseArg(arg, index): string { default: (typeAnnotation.name: empty); throw new Error( - `Unknown prop type for "${arg.name}, found: ${typeAnnotation.name}"`, + `Unknown prop type for "${arg.name}", found: "${typeAnnotation.name}"`, ); } case 'StringTypeAnnotation': @@ -91,6 +91,7 @@ function traverseArg(arg, index): string { case 'FunctionTypeAnnotation': return `std::move(${wrap('.getObject(rt).getFunction(rt)')})`; case 'GenericObjectTypeAnnotation': + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'ObjectTypeAnnotation': return wrap('.getObject(rt)'); case 'AnyTypeAnnotation': @@ -99,7 +100,7 @@ function traverseArg(arg, index): string { // TODO (T65847278): Figure out why this does not work. // (typeAnnotation.type: empty); throw new Error( - `Unknown prop type for "${arg.name}, found: ${typeAnnotation.type}"`, + `Unknown prop type for "${arg.name}", found: "${typeAnnotation.type}"`, ); } } diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js index a423127ee4..5de8f8ee3c 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleH.js @@ -74,6 +74,7 @@ function translatePrimitiveJSTypeToCpp( return 'int'; case 'BooleanTypeAnnotation': return 'bool'; + // case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': case 'ObjectTypeAnnotation': return 'jsi::Object'; diff --git a/packages/react-native-codegen/src/generators/modules/GenerateModuleHObjCpp.js b/packages/react-native-codegen/src/generators/modules/GenerateModuleHObjCpp.js index f5502f2a83..a2fd47e7c3 100644 --- a/packages/react-native-codegen/src/generators/modules/GenerateModuleHObjCpp.js +++ b/packages/react-native-codegen/src/generators/modules/GenerateModuleHObjCpp.js @@ -124,6 +124,7 @@ function translatePrimitiveJSTypeToObjCType( return nullable ? 'NSNumber *' : 'double'; case 'BooleanTypeAnnotation': return nullable ? 'NSNumber * _Nullable' : 'BOOL'; + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': return wrapIntoNullableIfNeeded('NSDictionary *'); case 'ArrayTypeAnnotation': diff --git a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js index ba0ce6a518..9be5bcbd53 100644 --- a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js +++ b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructs.js @@ -92,6 +92,7 @@ function getElementTypeForArray( return 'double'; case 'ObjectTypeAnnotation': return getNamespacedStructName(name, property) + 'Element'; + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': // TODO(T67565166): Generic objects are not type safe and should be disallowed in the schema. This case should throw an error once it is disallowed in schema. console.error( @@ -228,6 +229,7 @@ function getInlineMethodImplementation( return `RCTBridgingToBool(${element})`; case 'ObjectTypeAnnotation': return `${getNamespacedStructName(name, property)}Element(${element})`; + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': return element; case 'AnyObjectTypeAnnotation': diff --git a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructsForConstants.js b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructsForConstants.js index d43ab794e7..5d8dc29af7 100644 --- a/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructsForConstants.js +++ b/packages/react-native-codegen/src/generators/modules/ObjCppUtils/GenerateStructsForConstants.js @@ -96,6 +96,7 @@ function getBuilderInputFieldDeclaration( property.name, )}::Builder`, ); + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': case 'AnyTypeAnnotation': if (property.optional) { @@ -185,6 +186,7 @@ function getObjectProperty(property: ObjectParamTypeAnnotation): string { case 'BooleanTypeAnnotation': return boolGetter(property.name, property.optional); case 'StringTypeAnnotation': + case 'TypeAliasTypeAnnotation': // TODO: Handle aliases case 'GenericObjectTypeAnnotation': case 'AnyTypeAnnotation': return safeGetter(property.name, property.optional); diff --git a/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js index 3739531ef4..6feda94697 100644 --- a/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/generators/modules/__test_fixtures__/fixtures.js @@ -17,6 +17,7 @@ const EMPTY_NATIVE_MODULES: SchemaType = { SampleTurboModule: { nativeModules: { SampleTurboModule: { + aliases: {}, properties: [], }, }, @@ -29,6 +30,7 @@ const SIMPLE_NATIVE_MODULES: SchemaType = { SampleTurboModule: { nativeModules: { SampleTurboModule: { + aliases: {}, properties: [ { name: 'getConstants', @@ -291,6 +293,7 @@ const TWO_MODULES_SAME_FILE: SchemaType = { NativeSampleTurboModule: { nativeModules: { SampleTurboModule: { + aliases: {}, properties: [ { name: 'voidFunc', @@ -307,6 +310,7 @@ const TWO_MODULES_SAME_FILE: SchemaType = { ], }, Sample2TurboModule: { + aliases: {}, properties: [ { name: 'voidFunc', @@ -332,6 +336,7 @@ const TWO_MODULES_DIFFERENT_FILES: SchemaType = { NativeSampleTurboModule: { nativeModules: { SampleTurboModule: { + aliases: {}, properties: [ { name: 'voidFunc', @@ -352,6 +357,7 @@ const TWO_MODULES_DIFFERENT_FILES: SchemaType = { NativeSampleTurboModule2: { nativeModules: { Sample2TurboModule: { + aliases: {}, properties: [ { name: 'getConstants', @@ -390,6 +396,7 @@ const COMPLEX_OBJECTS: SchemaType = { NativeSampleTurboModule: { nativeModules: { SampleTurboModule: { + aliases: {}, properties: [ { name: 'difficult', diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js b/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js index e15a2f823f..a326be96ac 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/__test_fixtures__/fixtures.js @@ -154,12 +154,54 @@ type Num2 = Num; export type Void = void; export type A = number; export type B = number; +export type ObjectAlias = {| + x: number, + y: number, + label: string, + truthy: boolean, +|} export interface Spec extends TurboModule { // Exported methods. +getNumber: Num2; +getVoid: () => Void; +getArray: (a: Array) => {| a: B |}; + +getStringFromAlias: (a: ObjectAlias) => string; +} + +export default TurboModuleRegistry.getEnforcing('SampleTurboModule'); + +`; + +const NATIVE_MODULE_WITH_NESTED_ALIASES = ` +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + */ + +'use strict'; + +import type {TurboModule} from '../RCTExport'; +import * as TurboModuleRegistry from '../TurboModuleRegistry'; + +type Bar = {| + z: number +|}; + +type Foo = {| + bar1: Bar, + bar2: Bar, +|}; + +export interface Spec extends TurboModule { + // Exported methods. + foo1: (x: Foo) => void; + foo2: (x: Foo) => void; } export default TurboModuleRegistry.getEnforcing('SampleTurboModule'); @@ -459,6 +501,7 @@ module.exports = { NATIVE_MODULE_WITH_ARRAY_WITH_UNION_AND_TOUPLE, NATIVE_MODULE_WITH_FLOAT_AND_INT32, NATIVE_MODULE_WITH_ALIASES, + NATIVE_MODULE_WITH_NESTED_ALIASES, NATIVE_MODULE_WITH_PROMISE, NATIVE_MODULE_WITH_COMPLEX_OBJECTS, NATIVE_MODULE_WITH_COMPLEX_OBJECTS_WITH_NULLABLE_KEY, diff --git a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-test.js.snap b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-test.js.snap index ebfd9e4637..0aeb307d1b 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-test.js.snap +++ b/packages/react-native-codegen/src/parsers/flow/modules/__tests__/__snapshots__/module-parser-test.js.snap @@ -25,6 +25,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [], }, }, @@ -39,6 +40,41 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object { + "ObjectAlias": Object { + "properties": Array [ + Object { + "name": "x", + "optional": false, + "typeAnnotation": Object { + "type": "NumberTypeAnnotation", + }, + }, + Object { + "name": "y", + "optional": false, + "typeAnnotation": Object { + "type": "NumberTypeAnnotation", + }, + }, + Object { + "name": "label", + "optional": false, + "typeAnnotation": Object { + "type": "StringTypeAnnotation", + }, + }, + Object { + "name": "truthy", + "optional": false, + "typeAnnotation": Object { + "type": "BooleanTypeAnnotation", + }, + }, + ], + "type": "ObjectTypeAnnotation", + }, + }, "properties": Array [ Object { "name": "getNumber", @@ -104,6 +140,27 @@ Object { "type": "FunctionTypeAnnotation", }, }, + Object { + "name": "getStringFromAlias", + "typeAnnotation": Object { + "optional": false, + "params": Array [ + Object { + "name": "a", + "nullable": false, + "typeAnnotation": Object { + "name": "ObjectAlias", + "type": "TypeAliasTypeAnnotation", + }, + }, + ], + "returnTypeAnnotation": Object { + "nullable": false, + "type": "StringTypeAnnotation", + }, + "type": "FunctionTypeAnnotation", + }, + }, ], }, }, @@ -118,6 +175,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getArray", @@ -159,6 +217,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getArray", @@ -196,6 +255,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getArray", @@ -263,6 +323,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "passBool", @@ -358,6 +419,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getValueWithCallback", @@ -393,6 +455,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getArray", @@ -452,6 +515,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getObject", @@ -667,6 +731,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getConstants", @@ -768,6 +833,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getInt", @@ -817,12 +883,105 @@ Object { } `; +exports[`RN Codegen Flow Parser can generate fixture NATIVE_MODULE_WITH_NESTED_ALIASES 1`] = ` +Object { + "modules": Object { + "NativeSampleTurboModule": Object { + "nativeModules": Object { + "SampleTurboModule": Object { + "aliases": Object { + "Bar": Object { + "properties": Array [ + Object { + "name": "z", + "optional": false, + "typeAnnotation": Object { + "type": "NumberTypeAnnotation", + }, + }, + ], + "type": "ObjectTypeAnnotation", + }, + "Foo": Object { + "properties": Array [ + Object { + "name": "bar1", + "optional": false, + "typeAnnotation": Object { + "name": "Bar", + "type": "TypeAliasTypeAnnotation", + }, + }, + Object { + "name": "bar2", + "optional": false, + "typeAnnotation": Object { + "name": "Bar", + "type": "TypeAliasTypeAnnotation", + }, + }, + ], + "type": "ObjectTypeAnnotation", + }, + }, + "properties": Array [ + Object { + "name": "foo1", + "typeAnnotation": Object { + "optional": false, + "params": Array [ + Object { + "name": "x", + "nullable": false, + "typeAnnotation": Object { + "name": "Foo", + "type": "TypeAliasTypeAnnotation", + }, + }, + ], + "returnTypeAnnotation": Object { + "nullable": false, + "type": "VoidTypeAnnotation", + }, + "type": "FunctionTypeAnnotation", + }, + }, + Object { + "name": "foo2", + "typeAnnotation": Object { + "optional": false, + "params": Array [ + Object { + "name": "x", + "nullable": false, + "typeAnnotation": Object { + "name": "Foo", + "type": "TypeAliasTypeAnnotation", + }, + }, + ], + "returnTypeAnnotation": Object { + "nullable": false, + "type": "VoidTypeAnnotation", + }, + "type": "FunctionTypeAnnotation", + }, + }, + ], + }, + }, + }, + }, +} +`; + exports[`RN Codegen Flow Parser can generate fixture NATIVE_MODULE_WITH_NULLABLE_PARAM 1`] = ` Object { "modules": Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "voidFunc", @@ -858,6 +1017,20 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object { + "DisplayMetricsAndroid": Object { + "properties": Array [ + Object { + "name": "width", + "optional": false, + "typeAnnotation": Object { + "type": "NumberTypeAnnotation", + }, + }, + ], + "type": "ObjectTypeAnnotation", + }, + }, "properties": Array [ Object { "name": "getConstants", @@ -876,16 +1049,8 @@ Object { "name": "windowPhysicalPixels", "optional": false, "typeAnnotation": Object { - "properties": Array [ - Object { - "name": "width", - "optional": false, - "typeAnnotation": Object { - "type": "NumberTypeAnnotation", - }, - }, - ], - "type": "ObjectTypeAnnotation", + "name": "DisplayMetricsAndroid", + "type": "TypeAliasTypeAnnotation", }, }, ], @@ -915,16 +1080,8 @@ Object { "name": "windowPhysicalPixels", "optional": false, "typeAnnotation": Object { - "properties": Array [ - Object { - "name": "width", - "optional": false, - "typeAnnotation": Object { - "type": "NumberTypeAnnotation", - }, - }, - ], - "type": "ObjectTypeAnnotation", + "name": "DisplayMetricsAndroid", + "type": "TypeAliasTypeAnnotation", }, }, ], @@ -951,6 +1108,20 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object { + "SomeObj": Object { + "properties": Array [ + Object { + "name": "a", + "optional": false, + "typeAnnotation": Object { + "type": "StringTypeAnnotation", + }, + }, + ], + "type": "ObjectTypeAnnotation", + }, + }, "properties": Array [ Object { "name": "getValueWithPromise", @@ -1002,6 +1173,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getRootTag", @@ -1039,6 +1211,7 @@ Object { "NativeSampleTurboModule": Object { "nativeModules": Object { "SampleTurboModule": Object { + "aliases": Object {}, "properties": Array [ Object { "name": "getObject", diff --git a/packages/react-native-codegen/src/parsers/flow/modules/aliases.js b/packages/react-native-codegen/src/parsers/flow/modules/aliases.js new file mode 100644 index 0000000000..7c55e4d9ff --- /dev/null +++ b/packages/react-native-codegen/src/parsers/flow/modules/aliases.js @@ -0,0 +1,70 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + */ + +'use strict'; + +import type {ObjectTypeAliasTypeShape} from '../../../CodegenSchema.js'; + +import type {TypeMap} from '../utils.js'; + +const {getObjectProperties} = require('./properties'); + +// $FlowFixMe there's no flowtype for ASTs +type MethodAST = Object; + +function getAliases( + typeDefinition: $ReadOnlyArray, + types: TypeMap, +): $ReadOnly<{[aliasName: string]: ObjectTypeAliasTypeShape, ...}> { + const aliases = {}; + typeDefinition.map(moduleAlias => { + const aliasName = Object.keys(moduleAlias)[0]; + const typeAnnotation = moduleAlias[Object.keys(moduleAlias)[0]]; + + switch (typeAnnotation.type) { + case 'ObjectTypeAnnotation': + aliases[aliasName] = { + type: 'ObjectTypeAnnotation', + ...(typeAnnotation.properties && { + properties: getObjectProperties( + aliasName, + {properties: typeAnnotation.properties}, + aliasName, + types, + ), + }), + }; + return; + case 'GenericTypeAnnotation': + if (typeAnnotation.id.name && typeAnnotation.id.name !== '') { + aliases[aliasName] = { + type: 'TypeAliasTypeAnnotation', + name: typeAnnotation.id.name, + }; + return; + } else { + throw new Error( + `Cannot use "${typeAnnotation.type}" type annotation for "${aliasName}": must specify a type alias name`, + ); + } + default: + // TODO (T65847278): Figure out why this does not work. + // (typeAnnotation.type: empty); + throw new Error( + `Unknown prop type, found "${typeAnnotation.type}" in "${aliasName}"`, + ); + } + }); + return aliases; +} + +module.exports = { + getAliases, +}; diff --git a/packages/react-native-codegen/src/parsers/flow/modules/index.js b/packages/react-native-codegen/src/parsers/flow/modules/index.js index b610dc30e4..c09c66acf4 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/index.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/index.js @@ -11,6 +11,7 @@ 'use strict'; import type {NativeModuleSchemaBuilderConfig} from './schema.js'; +const {getAliases} = require('./aliases'); const {getMethods} = require('./methods'); function getModuleProperties(types, interfaceName) { @@ -33,13 +34,41 @@ function findInterfaceName(types) { )[0].id.name; } +function findAliasNames(types) { + return Object.keys(types) + .map(typeName => types[typeName]) + .filter( + type => + type.type && + type.type === 'TypeAlias' && + type.right && + type.right.type === 'ObjectTypeAnnotation', + ) + .map(type => type.id.name); +} + +function getModuleAliases(types, aliasNames) { + return aliasNames.map(aliasName => { + if (types[aliasName] && types[aliasName].right) { + return {[aliasName]: types[aliasName].right}; + } + throw new Error( + `Interface properties for "${aliasName}" has been specified incorrectly.`, + ); + }); +} + // $FlowFixMe there's no flowtype for AST function processModule(types): NativeModuleSchemaBuilderConfig { const interfaceName = findInterfaceName(types); - const moduleProperties = getModuleProperties(types, interfaceName); const properties = getMethods(moduleProperties, types); - return {properties}; + + const aliasNames = findAliasNames(types); + const moduleAliases = getModuleAliases(types, aliasNames); + const aliases = getAliases(moduleAliases, types); + + return {aliases, properties}; } module.exports = { diff --git a/packages/react-native-codegen/src/parsers/flow/modules/methods.js b/packages/react-native-codegen/src/parsers/flow/modules/methods.js index 37bc10758d..c63fc90369 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/methods.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/methods.js @@ -85,6 +85,16 @@ function getTypeAnnotationForParam( ); } case 'ObjectTypeAnnotation': + if (param.typeAnnotation.type === 'GenericTypeAnnotation') { + return { + nullable, + name: paramName, + typeAnnotation: { + type: 'TypeAliasTypeAnnotation', + name: param.typeAnnotation.id.name, + }, + }; + } return { nullable, name: paramName, diff --git a/packages/react-native-codegen/src/parsers/flow/modules/properties.js b/packages/react-native-codegen/src/parsers/flow/modules/properties.js index bd7b83a7dc..f1e8c51e7f 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/properties.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/properties.js @@ -13,6 +13,7 @@ import type { FunctionTypeAnnotationParamTypeAnnotation, ObjectParamTypeAnnotation, + TypeAliasTypeAnnotation, } from '../../../CodegenSchema.js'; import type {ASTNode, TypeMap} from '../utils.js'; @@ -53,7 +54,10 @@ function getElementTypeForArrayOrObject( arrayParam: ASTNode, paramName: string, types: TypeMap, -): FunctionTypeAnnotationParamTypeAnnotation | typeof undefined { +): + | FunctionTypeAnnotationParamTypeAnnotation + | TypeAliasTypeAnnotation + | typeof undefined { const typeAnnotation = getValueFromTypes(arrayParam, types); const type = typeAnnotation.type === 'GenericTypeAnnotation' @@ -83,10 +87,16 @@ function getElementTypeForArrayOrObject( }; } else { throw new Error( - `Unsupported type for ${name}, param: "${paramName}": expected to find annotation for type of nested array contents`, + `Unsupported type for "${name}", param: "${paramName}": expected to find annotation for type of nested array contents`, ); } case 'ObjectTypeAnnotation': + if (arrayParam.id) { + return { + type: 'TypeAliasTypeAnnotation', + name: arrayParam.id.name, + }; + } return { type: 'ObjectTypeAnnotation', properties: getObjectProperties(name, typeAnnotation, paramName, types), diff --git a/packages/react-native-codegen/src/parsers/flow/modules/schema.js b/packages/react-native-codegen/src/parsers/flow/modules/schema.js index b2da0d43dd..68b1d61c98 100644 --- a/packages/react-native-codegen/src/parsers/flow/modules/schema.js +++ b/packages/react-native-codegen/src/parsers/flow/modules/schema.js @@ -12,15 +12,17 @@ import type { SchemaType, + ObjectTypeAliasTypeShape, NativeModuleMethodTypeShape, } from '../../../CodegenSchema.js'; export type NativeModuleSchemaBuilderConfig = $ReadOnly<{| + aliases: $ReadOnly<{[aliasName: string]: ObjectTypeAliasTypeShape, ...}>, properties: $ReadOnlyArray, |}>; function buildModuleSchema( - {properties}: NativeModuleSchemaBuilderConfig, + {aliases, properties}: NativeModuleSchemaBuilderConfig, moduleName: string, ): SchemaType { return { @@ -28,6 +30,7 @@ function buildModuleSchema( [`Native${moduleName}`]: { nativeModules: { [moduleName]: { + aliases, properties, }, },