Chore/upgrade from upstream #1
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewed By: priteshrnandgaonkar Differential Revision: D9943869 fbshipit-source-id: 6b6550f453ab4e0ab8305401b299f2d0ed415f72
Summary: Part of: react-native-community/discussions-and-proposals#29 This PR removes all but one instance of PropTypes in `RNTester`. The last remaining conversion is `CameraRollView`, which I will do in a separate PR. Pull Request resolved: facebook#21321 Differential Revision: D10041809 Pulled By: TheSavior fbshipit-source-id: c03b1ce5ad640ae59ae6240a3b6c13581345b5a3
Summary: This PR is the result of running `yarn prettify` on the codebase - which caught a few files that were not prettified. This will make instructing people to run prettify a bit less complicated, since unrelated files will not show up in diffs. Pull Request resolved: facebook#21327 Differential Revision: D10046057 Pulled By: TheSavior fbshipit-source-id: 2c771a3c758c72816c707e32ee2f4587e466f277
Summary: This also fixes an issue with tagged commit Circle CI jobs, and avoids running the `publish_npm_package` on every commit. The new config ignores commits on *all branches*, but should still catch git tags. Pull Request resolved: facebook#21324 Differential Revision: D10041629 Pulled By: hramos fbshipit-source-id: 9b3295b5fcd614c67a8838ffd49c6a5d6ae7fd86
Summary: Follow up to facebook/metro@8932a9c Babel helpers and regenerator runtime will be imported automatically from `babel/runtime`. We still need to add the global regeneratorRuntime for jest tests since we disable babel-runtime currently. Pull Request resolved: facebook#21283 Reviewed By: mjesun Differential Revision: D10010963 Pulled By: rafeca fbshipit-source-id: da5e23dd901f8f8940d46816b4fc9290d0e28c76
Summary: Now we have a way to enumerate surfaces stored in RCTSurfaceRegistry. Reviewed By: mdvacca Differential Revision: D9931316 fbshipit-source-id: 6b886c4b52cebddf15fef0239190fb2730d682cc
Summary: UIManager now can install and uninstall itself calling a functions that are provided as constructor arguments. Reviewed By: mdvacca Differential Revision: D9931329 fbshipit-source-id: b8d2d9925b0e2db0fed44bdf2e185d198fabd5ee
Summary: All integration with Bridge was removed from RCTFabricSurface, now it's Surface's responsibility to start and stop JS app and register the ShadowTree in the Scheduler. Reviewed By: mdvacca Differential Revision: D9931317 fbshipit-source-id: 55a682f0afb1c542a904e1a8570029e4690967cc
Summary: Now RCTSurfacePresenter is uniquely responsible for: * Starting and stopping JS apps; * Restarting JS apps during hot-reload; * Recreating Scheduler during hot-reload. Reviewed By: mdvacca Differential Revision: D9931318 fbshipit-source-id: a6a3fb58814222f71cc6cb2caad620ed6319089d
…s method Summary: We call this method in a constructor before the actual object is beeing constructed, so it's incorrect; it should be class method. Reviewed By: mdvacca Differential Revision: D9931315 fbshipit-source-id: 304ba8e2354f3f408cfa2bf1729266525a08f951
Summary: The original design of RCTSurface implied that the Surface starts on initialization and stops on deallocation. Recently I realized that this not sufficient API in some cases when the application uses ARC with autorelease pools (that can postpone object deallocations, which is highly undesirable). And that's simply handy to have those methods sometimes. Reviewed By: mdvacca Differential Revision: D9982356 fbshipit-source-id: baa3bd24804d3708606ebd00b8795f2d5c9d4de9
Summary: Besides that it's more simple and straight-forward now, we need that to always instantiate Scheduler with a context full of fresh valid objects derived from the new instance of the bridge. Reviewed By: mdvacca Differential Revision: D9995780 fbshipit-source-id: 534a314152d93562b08dd7857962f174b0d06886
Summary: EventBeatBasedExecutor is an executor derived from EventBeat and using EventBeat to ensure proper threading. Why do we need yet another executor? Because otherwise, we have to make it platform specific-dependency that each platform-specific implementation has to implement and provide. We already have all that we need in already provided EventBeat, so we can just convert that into simple executor in a platform-agnostic way. Reviewed By: mdvacca Differential Revision: D9995783 fbshipit-source-id: f8aa72a9744e50ebecbea9ad0e2546f41f5358f2
…lling UIManager Summary: This is the last step before making JSIUIManagerInstaller a direct dependency of UIManager (and making UIManager installation process completely seamless/platform-agnostic). Reviewed By: mdvacca Differential Revision: D9995781 fbshipit-source-id: 6f8c7177495b01ebaac1dbe330f49dce2e2a552c
Summary: There is no need to make JS calls to start or stop React Native apps; Scheduler does it automatically. Yay! With this change (because we have to change Scheduler API) we are starting slow process migrating away from using term `reactRootTag` when we refer to running a ReactNative app (aka Surface). We will use `surfaceId` instead. We plan to slowly and gracefully retire `reactTag` term entity replacing it with several appropriate entities specific for particular usage, e.g. `viewId` (some id which makes sense for mounting), `surfaceId`, `nodeId` (unique id representing nodes which were cloned from the original one), or `eventTarget`. Reviewed By: mdvacca Differential Revision: D9999336 fbshipit-source-id: bbc7303c195b070b8c235c9ca35546d1dc693e98
Summary: Instead of wrapping all public methods into The controller you requested could not be found. blocks/mutexes, RCTSurfacePresenter utilizes a different thread-safety pattern where all instance variable are granularly thread-safe. The names of all internal methods were prefixed by '_'. Reviewed By: mdvacca Differential Revision: D10033407 fbshipit-source-id: 97fd2879c879dd9ef8d9ece24e25af00f749a871
Summary: We have to uninstall UIManager synchronously to avoid a race condition when JS is capable to call already deallocated UIManager. Reviewed By: mdvacca Differential Revision: D10033406 fbshipit-source-id: 194d1ae2dd5ab09b036b1c165de289ada8e66014
Summary: Apparently, after updating CALayer props we have to request redrowing on top of it manually. Reviewed By: mdvacca Differential Revision: D10053340 fbshipit-source-id: f87311399bab809c9e13a3076f526bbe3f7f3fb4
…9567) Summary: Fixes facebook#18223 This is a fairly simple solution to what seems to be a recurring issue where certain requests that result in an empty body where JSON is expected throw an error rather than being handled gracefully. Client side error handling is not being hit as this is being thrown at a lower level. Make a http request that results in an empty blob: "" [INTERNAL] [BUGFIX] [XMLHttpRequest.js] - Line 262 Pull Request resolved: facebook#19567 Differential Revision: D8314416 Pulled By: hramos fbshipit-source-id: a17c49f3620f0abbb936f3a1c2b01aa1b64820fd
Summary: Don't ask. Really, all those descriptions from official docs like below are useless: * `__bridge_transfer` Moves a Core Foundation pointer to Objective-C with transfer of the ownership to ARC. * `__bridge` Transfers a pointer between Objective-C and Core Foundation with no transfer of ownership All that is totally confusing and useless. At the end of the day, we only have to think about which additional `CFRetain` and `CFRelease` ARC will add or will not add for our pointers. So, following official docs recommendation, we would like to add `__bridge_transfer` because of course, we do want to ARC managing the variable after we introduced it to ARC here. But we also want to have shared ownership of this. That's the key. If we use `__bridge_transfer` ARC will assume that this variable already retained once (because it exists) and will call CFRelease at the end of the scope. Right before that when we pass this variable down to call stack ARC will retain and then manage the variable according to the rest of the code. But still, from this point, we will have zero-balanced reference counter; the owning by `shared_ptr` bump is already compensated with `CFRelease` at the end of the scope. As soon as the rest of the code release the object, it will be incorrectly deallocated. So, instead of using `__bridge_transfer` we have to use `__bridge`. That will indicate that *in this block* ARC does not manage the reference counter of the variable (which is kinda true because having `shared_ptr` inside the block already retains that) and will not add `CFRelease` at the end of the block. Reviewed By: mdvacca Differential Revision: D10054241 fbshipit-source-id: 6e82c5270fe5d53f1ed68e167b94f70dc4367a9f
Summary: Trivial. `imageLocalData` retains a network request, observers and actual bitmaps. Reviewed By: mdvacca Differential Revision: D10054430 fbshipit-source-id: 9bea11677b73e9e7ce7bc50bd14ec5515dac60de
Summary: This diff removes the `convertNewToOld` config method, which was only used by the unit tests (since there's no need to convert a new config object into an old one). As part of this, the config tests have been tweaked and simplified a bit, while keeping similar assertions and adding some more checks. Reviewed By: mjesun Differential Revision: D10015079 fbshipit-source-id: ba4fbd09cd4f97168bc1a2dbcec2699a00243637
Summary: This makes the Metro config type readonly, only a couple of things inside Metro needed to be tweaked :) Reviewed By: mjesun Differential Revision: D10028083 fbshipit-source-id: 15f5d957a8ee7384d6156c973639d86240fd251f
…1326) Summary: This PR adds these two scripts to `package.json`: * `flow-check-ios` - for running `flow check` on `.ios.js` files * `flow-check-android` - for running `flow check` on `.android.js` files The Android command makes use of the new `--flowconfig-name` option added to Flow in `0.80.0` Pull Request resolved: facebook#21326 Differential Revision: D10055702 Pulled By: hramos fbshipit-source-id: e647f8d2995da0a9bd6af242218819fd5745df63
Summary: During the JS run, the view managers used on a React Native screen eventually call the native methods for `UIManagerModule.getConstantsForViewMangers(viewManagerName)`. This blocks the JS thread. This diff tries to cache the values of those calls and return them when JS needs it, ensuring that JS is not blocked as much. Reviewed By: achen1 Differential Revision: D9985817 fbshipit-source-id: 36feabc8a386956f8a6474f6e7978285d31f24dd
Summary: This PR moves `DeprecatedViewPropTypes` to a new `DeprecatedViewProps` folder, and copies all documentation comments to the relevant Flow types file. Pull Request resolved: facebook#21349 Differential Revision: D10080802 Pulled By: TheSavior fbshipit-source-id: af4881f3b12e8a1e675b849e0fcf0cc57a68e57f
Summary: This PR split PointPropType.js into PointPropType.js with Flow definition and Libraries/DeprecatedPointPropType.js remaining with PropTypes definition. Related to facebook#21342 Pull Request resolved: facebook#21355 Differential Revision: D10081399 Pulled By: TheSavior fbshipit-source-id: 2283ff3fbda6b0f525742336f92fd6279250b874
Summary: Related to facebook#21342 Pull Request resolved: facebook#21350 Differential Revision: D10081454 Pulled By: TheSavior fbshipit-source-id: db27a1f23c643b7d6d73136254eff91625419583
Summary: related facebook#21342 This is a first PR for this repo. So, if there are any problem, please tell me 🙇 TODO * delete props types * apply read only interface CheckList - [x] `yarn prettier` - [x] `yarn flow-check-ios` Pull Request resolved: facebook#21346 Differential Revision: D10081962 Pulled By: TheSavior fbshipit-source-id: 32387c58f180b9aa5f854e323a4bb29aa73f04c8
Summary: Related to facebook#21342 Pull Request resolved: facebook#21345 Differential Revision: D10081976 Pulled By: TheSavior fbshipit-source-id: d6a905704fc5c2f10a6a8552f04e9c3feaeb147b
Summary: change RCTCxxBridge to use JSIExecutorFactory+JSCRuntime instead of JSCExecutorFactory. Also remove JSC usage from RN in other files. This allows deleting files, too, which is done further down the stack. Reviewed By: RSNara Differential Revision: D9369111 fbshipit-source-id: 67ef3146e3abe5244b6cf3249a0268598f91b277
Summary: JSI doesn't use any of this. Reviewed By: RSNara Differential Revision: D10229167 fbshipit-source-id: 9eaa288a1d62bafb3ff0626f9f8430e699fdad4a
Summary: JSI+JSCRuntime replaces direct use of JSC @public Reviewed By: danzimm Differential Revision: D9328235 fbshipit-source-id: a4d0cad8250ef2da058ffffbdedbffa19f96bb12
Summary: JSI+JSCRuntime replaces direct use of JSC. This is like the previous diff, except for iOS. Reviewed By: RSNara Differential Revision: D9369108 fbshipit-source-id: 4ed2c0d660ba2a30edf699d95278c72cabcc9203
Summary: This diff includes a few changes: 1. Move the headers inside `jsiexecutor` into `jsiexecutor/jsireact`. As far as I'm aware, the Android ndk build system isn't flexible enough to support header namespaces, so we can't just expose the headers inside the `jsiexecutor` directory under the `jsireact` namespace. Therefore, I moved the headers to `jsiexecutor/jsireact`, and added `jsiexecutor` to the header search path. This was the easiest way to simulate `jsireact` namespace. 2. Setup the Android.mk files to get RNTester compiling and running. 3. Introduce a `jscexecutor` module to make `JSCExecutor.java` execute without throwing. **Note:** Moving the header files inside `jsiexecutor` probably breaks the iOS builds and internal builds. I'll fix those in subsequent diffs on this stack. Reviewed By: shergin Differential Revision: D9995429 fbshipit-source-id: 418a4ee91f585842c5e317af2f300227a51e9ba8
Summary: This diff includes a few changes to the `React.podspec` file: 1. Introduce a `jsi` spec for code inside the `ReactCommon/jsi` folder. This depends on the JavaScriptCore framework. 2. Introduce a `jsiexecutor` spec for the code inside the `ReactCommon/jsiexecutor` folder. These files depend on files in `ReactCommon/cxxreact`, `ReactCommon/jsi`, and Folly. 3. Since RCTCxxBridge.mm now depends on `JSIExecutor`, we need to have the `CxxBridge` spec depend on the `jsiexecutor` spec. Reviewed By: hramos Differential Revision: D9820323 fbshipit-source-id: 0c96d027eed30ee47b6ee0d2d86cd6b1ad7a5887
Summary: Marc deleted a few files from react-native-github, so I removed them from the RNTester XCode project. I also included the files he created, and created new targets: `jsiexecutor-tvOS`, `jsiexecutor`, `jsi`, `jsi-tvOS`. **Note:** The tvOS build of RNTester is broken in this diff because of a few `WKWebView` changes I landed earlier. D9844322 includes the fix. Reviewed By: axe-fb Differential Revision: D9875409 fbshipit-source-id: 31a9f241a524de91e78dfff0555aec5d1373d789
Summary: The only thing extra that we need to do is to include `JavaScriptCore.framework` inside the HelloWorld.xcodeproj file. Reviewed By: hramos Differential Revision: D9893035 fbshipit-source-id: 2a29d1fd645eafa2e09109ad14d09f812dfa2601
Reviewed By: fkgozali Differential Revision: D10441260 fbshipit-source-id: 5a77ec382e28be046824bd598186e6c29a1510f2
Summary:
`extern "C"` disables name mangling, hence input parameter types does not influence the name. That makes it impossible to have several equality operators with `extern "C"` linkage (for different types).
One such operator is defined in Windows SDK, in `guiddef.h`. It in turn is included in `winnt.h` inside `extern "C" { ... }` block. Trying to compile file which both is dependent both on `winnt.h` and `Yoga.h` results in:
```
Yoga.h(50): error C2733: 'operator ==': second C linkage of overloaded function not allowed
guiddef.h(192): note: see declaration of 'operator =='
```
In general it doesn't make much sense to have cpp specific operator to have `extern "C"` linkage, so the change doesn't introduce any controlling flag (mangling on/off).
Note that it's breaking binary compatibility and yoga library should be rebuilt if those operators are used.
Reviewed By: milend
Differential Revision: D10418395
fbshipit-source-id: 2f1cccff26165e638b9a07eece07d94fccfa5e5a
Summary: This diff just builds on top of the open source PR: 1. I add a bunch of extra flow typings to the file. 2. I refactor some of the JavaScript code. Reviewed By: TheSavior Differential Revision: D10351693 fbshipit-source-id: a6d828518150c11d66a179c5c3fe835cc80a8dfb
) Summary: Related to facebook#21485 (comment) Remove createReactClass and TimerMixin from IntegrationTests/TimersTest.js Pull Request resolved: facebook#21748 Reviewed By: TheSavior Differential Revision: D10366418 Pulled By: RSNara fbshipit-source-id: f7e9a1b62a17cd23374997f99dbfe239172b614f
Summary: @public If you call NetInfo.getCurrentConnectivity multiple times in succession, we'll create a bunch of callbacks but lose them in the ether. With this fix, we'll unschedule them before creating a new one, which should resolve some crashes we're seeing. Reviewed By: PeteTheHeat Differential Revision: D10409486 fbshipit-source-id: 6065b09fa626f7f06aed9bf0e278c0a6a6169f58
Summary: Related to facebook#21581 Remove createReactClass from CameraRollView. Reviewed By: TheSavior Differential Revision: D10351036 fbshipit-source-id: 394545ac143917e3b483dfc6186e5f45732c602a
Summary: This diff includes: 1. Touchups to the `CameraRollView` typings. 2. Typings for `CameraRollViewExmaple`. 3. Flow fixes for internal callsites. Reviewed By: yungsters Differential Revision: D10362686 fbshipit-source-id: 48bf3fba0566e9c5c062aee3342d669f6c143d9f
Summary:
We don't want people requiring from this file via haste. This file is the main for the react-native yarn workspace requireable via require('react-native').
https://pxl.cl/jnWC
Reviewed By: mostafaeweda, RSNara
Differential Revision: D10444200
fbshipit-source-id: 3832857e3df01def128525f32c9735e928802b59
…ain thread Summary: [RN] Relax the requirement that lazy module cannot be initialized on the main thread I tried to understand the D5364734 that intoduced this, and I am not sure, but belive that asserting here is too strict. If you have a module that you want to lazily initialize, and module does not demand the main queue, it should be just a warning if you run on the main queue, not necessarily an error. Reviewed By: mmmulani Differential Revision: D10429880 fbshipit-source-id: 018c211d45b98dd8c552bf0289fe517d05e56d47
…ook#21849) Summary: A while back Jest introduced `jest.requireActual` and `jest.requireMock` which are aliases to `require.requireActual` and `require.requireMock`. We believe that users should use official Jest API and are planning to deprecate the latter. Pull Request resolved: facebook#21849 Differential Revision: D10448849 Pulled By: TheSavior fbshipit-source-id: 34fffde97f48c26098c74ee222a56d99071703a6
Summary: ListView is deprecated and SwipeableListView uses ListView. Thus, it is deprecated as well. Reviewed By: RSNara Differential Revision: D10437408 fbshipit-source-id: a08391d5b099e74b6ec179cd940ac404b2e702f4
|
|
ontzic
pushed a commit
to ontzic/react-native
that referenced
this pull request
Mar 28, 2019
Summary: In D14571128, we made it so that when a JS object's property was `undefined`, we wouldn't insert that property into the corresponding NSDictionary. Here are two important observations about that diff: 1. ALL JS `null`s were now being converted to `NSNull`, and JS `undefined`s were now being converted to `nil`. 2. If a JS object's property was explicitly `null`, then we'd insert `NSNull` into the corresponding dictionary. Considering that when a property doesn't exist in a `NSDictionary`, property access returns `nil`, I've made it so that if a JS object's property is either `null` or `undefined`, then we simply do not insert it in the corresponding `NSDictionary`. Also, I've reverted microsoft#1 and made it so that `undefined` and `null` always map to the ObjC `nil`. This shouldn't unfix the problem that D14571128 was trying to fix. Here's my understanding of the problem that D14571128 was trying to fix (to make sure I'm not breaking something by this diff). This method was invoked from JS. ``` RCT_EXPORT_METHOD(logEvents:(NSDictionary *)events) { RCTAssert(events, @"You should provide events for logger"); [[NSNotificationCenter defaultCenter] postNotificationName:@"FBReactPerfLoggerDidReceiveEventsNotification" object:nil userInfo:@{@"FBReactPerfLoggerUserInfoPerfEventsKey" : [events copy]}]; } ``` The above dispatch calls into this method, which appends `events` into `_pendingJSPerfEvents`. ``` - (void)reactPerfLoggerDidReceiveEvents:(NSNotification *)notification { NSDictionary *events = notification.userInfo[@"FBReactPerfLoggerUserInfoPerfEventsKey"]; if (events) { dispatch_async(_eventQueue, ^{ if (self->_sessionData.perfLoggerFlagId != nil) { if ([self processJSPerfEvents:events]) { [self reportMetricsIfFinished]; } } else { [self->_pendingJSPerfEvents addObject:events]; } }); } } ``` Then, in `_processJSPerfEvents`, we do the following (link: https://fburl.com/tr4wr2a7): ``` NSNumber *actionId = events[@"actionId"]; if (actionId) { self->_sessionData.actionId = actionId; } ``` So, if `undefined` or `null` was passed in as the `actionId` property of the `events` JS object in `FBReactPerformanceLogger logEvents:`, then we'd default the `NSDictionary` to have `NSNull` in the corresponding property. This is bad because we had this line in FBReactWildePerfLogger (link: https://fburl.com/2nsywl2n): `actionId ? [actionId shortValue] : PerfLoggerActions.SUCCESS`. Essentially, this is the same problem that my diff is trying to fix. Reviewed By: fkgozali Differential Revision: D14625287 fbshipit-source-id: c701d4b6172484cee62494256175e8b205b23c73
NickGerleman
referenced
this pull request
in NickGerleman/react-native
Nov 6, 2019
Summary: In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed. Attempt #1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger. Attempt #2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile. At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv). Reviewed By: amir-shalem Differential Revision: D17470379 fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2
NickGerleman
referenced
this pull request
in NickGerleman/react-native
Nov 6, 2019
…ent #1 Summary: We need to migrate to HostComponent, this is the first batch. Changelog: [Internal] Migrate NativeComponentType from codegenNativeComponent to HostComponent Reviewed By: rickhanlonii Differential Revision: D17562879 fbshipit-source-id: ce1993b64a79cede3598c89ddff0dadf07fde92f
ZihanChen-MSFT
added a commit
that referenced
this pull request
Aug 26, 2020
* Add ScreenshotManagerTurboModule, but it is not called. * ... * Update AppDelegate.mm * Fix code due to facebook API rename * Update AppDelegate.mm * [Pods] Expose boost headers to consumer of RCT-Folly (#1) * Update AppDelegate.mm * ScreenshotManagerTurboModule gets called * reject the promise in takeScreenshot * ... * Put TurboModuleRegistry.get call at the right place * ... * Successfully took the screenshot * Remove unnecessary code * Fix code review comments * Fix code review comments * ... * Fix code review comments * Fix build break * Fix build break * Fix build break * Update NativeScreenshotManager.js * Fix build break * Fix yarn lint errors Co-authored-by: Eloy Durán <[email protected]>
NickGerleman
referenced
this pull request
in NickGerleman/react-native
Aug 28, 2020
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
NickGerleman
referenced
this pull request
in NickGerleman/react-native
Nov 12, 2020
Summary:
changelog: [internal]
Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t
# Why is using size_t better when working with indexes.
## 1. Type conversion isn't for free.
Take this example
```
size_t calculate(int number) {
return number + 1;
}
```
It generates following assembly (generated with armv8-a clang 10.0.0):
```
calculate(int): // calculate(int)
sub sp, sp, microsoft#16 // =16
str w0, [sp, microsoft#12]
ldr w8, [sp, microsoft#12]
add w9, w8, #1 // =1
mov w8, w9
sxtw x0, w8
add sp, sp, microsoft#16 // =16
ret
```
That's 9 instructions.
If we get rid of type conversion:
```
size_t calculate(size_t number) {
return number + 1;
}
```
Assembly (generated with armv8-a clang 10.0.0):
```
calculate(unsigned long): // calculate(unsigned long)
sub sp, sp, microsoft#16 // =16
str x0, [sp, microsoft#8]
ldr x8, [sp, microsoft#8]
add x0, x8, #1 // =1
add sp, sp, microsoft#16 // =16
ret
```
Compiler now produces only 7 instructions.
## Semantics
When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.
Reviewed By: JoshuaGross
Differential Revision: D24332248
fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
Saadnajmi
referenced
this pull request
in Saadnajmi/react-native-macos
Mar 11, 2021
Merge upstream into fork
christophpurrer
pushed a commit
to christophpurrer/react-native-macos
that referenced
this pull request
Nov 25, 2023
Summary: Pull Request resolved: facebook#41466 ## Context In open source, all apps use the same turbomodulemanager delegate (i.e: the default delegate). This diff introduces the buck infra that makes the oss default delegate work for meta apps. Concretely, we are going to make React Native use the same delegate for **all** Meta apps. Each Meta app will: 1. At build time, generate a unique TMProvider map 2. At app init time, initialize the default delegate with the TMProvider map. ## Implementation **Step microsoft#1:** At build time, generate a unique TMProvider map **Insight:** Buck genrules can accept, as input, the output of a buck query. So, here's how we get this done: 1. Buck query (i.e: input to Genrule): Given the app's deps, query all the schemas in the app. 2. Genrule: Read the schemas to generate the TMProvider map. The TMProvider map will also contain **all** the app's C++ module codegen. Concretely: 1. This diff introduces a macro: rn_codegen_appmodules(deps). 2. rn_codegen_appmodules(deps) generates appmodules.so, which contains the TMProvider map. **Step microsoft#2:** At app init time, initialize the default delegate with the TMProvider map. This is how we'll initialize the DefaultTurboModuleManagerDelegate: 1. DefaultTurboModuleManagerDelegate will load appmodules.so during init. 2. When loaded, appmodules.so will assign the code-generated TMProvider map to DefaultTurboModuleManagerDelegate. ## Impact This should allow us to: 1. Get one step closer to getting rid of the `js1 build turbomodule-manager-delegates --target <app>` script 3. Remove the TurboModuleManagerDelegate from React Native's public API. (Because we use one delegate for all React Native apps in Meta and OSS) Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D50988397 fbshipit-source-id: 0ca5dec14e2dae89ec97f5d39a182c7937c5c7bf
christophpurrer
pushed a commit
to christophpurrer/react-native-macos
that referenced
this pull request
Feb 14, 2024
…gets Summary: Changelog: [Internal] # This diff 1. Provides all Targets with an `executorFromThis()` method, which can be used from within a Target to access a *`this`-scoped main thread executor* = a `std::function` that will execute a callback asynchronously iff the current Target isn't destroyed first. 2. Refactors how (all) Target objects are constructed and retained, from a plain constructor to `static shared_ptr create()`. This is because `executorFromThis()` relies internally on `enable_shared_from_this` plus two-phase construction to populate the executor. 3. Creates utilities for deriving scoped executors from other executors and `shared_ptr`s. The concept is very much like `RuntimeExecutor` in reverse: the microsoft#1 use case is moving from the JS thread back to the main thread - where "main thread" is defined loosely as "anywhere it's legal to call methods on Target/Agent objects, access session state, etc". The actual dispatching mechanism is left up to the owner of each `PageTarget` object; for now we only have an iOS integration, where we use `RCTExecuteOnMainQueue`. Coupling the ownership/lifetime semantics with task scheduling is helpful, because it avoids the footgun of accidentally/nondeterministically moving `shared_ptr`s (and destructors!) to a different thread/queue . # This stack I'm refactoring the way the Runtime concept works in the modern CDP backend to bring it in line with the Page/Instance concepts. Overall, this will let us: * Integrate with engines that require us to instantiate a shared Target-like object (e.g. Hermes AsyncDebuggingAPI) in addition to an per-session Agent-like object. * Access JSI in a CDP context (both at target setup/teardown time and during a CDP session) to implement our own engine-agnostic functionality (`console` interception, `Runtime.addBinding`, etc). * Manage CDP execution contexts natively in RN, and (down the line) enable first-class debugging support for multiple Runtimes in an Instance. The core diffs in this stack: * ~~Introduce a `RuntimeTarget` class similar to `{Page,Instance}Target`. ~~ * ~~Make runtime registration explicit (`InstanceTarget::registerRuntime` similar to `PageTarget::registerInstance`). ~~ * ~~Rename the existing `RuntimeAgent` interface to `RuntimeAgentDelegate`.~~ * ~~Create a new concrete `RuntimeAgent` class similar to `{Page,Instance}Agent`.~~ * ~~Provide `RuntimeTarget` and `RuntimeAgent` with primitives for safe JSI access, namely a `RuntimeExecutor` for scheduling work on the JS thread.~~ * Provide RuntimeTarget with mechanism for scheduling work on the "main" thread from the JS thread, for when we need to do more than just send a CDP message (which we can already do with the thread-safe `FrontendChannel`) in response to a JS event. *← This diff* ## Architecture diagrams Before this stack: https://pxl.cl/4h7m0 After this stack: https://pxl.cl/4h7m7 Reviewed By: hoxyq Differential Revision: D53356953 fbshipit-source-id: 152c784eb64e9b217fc2966743b33f61bd8fd97e
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[GENERAL][CHORE] - Upgrade with upstream changes from facebook.