Summary:
If name is passed in as part of RuntimeConfig, that is included
in the description, too.
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D20716320
fbshipit-source-id: f2fba6df32f496090dee787d8b7f55a6a4dd8ed8
Summary:
`fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency.
Changelog: [Internal]
Reviewed By: scottrice
Differential Revision: D20495655
fbshipit-source-id: a57b72f694c533e2e16dffe74eccb8fdec1f55f5
Summary:
This was causing an exception cascade leading to production
errors. Added a test which repros the problem and passes with the
fix.
Changelog: [Internal]
Reviewed By: tmikov
Differential Revision: D20408858
fbshipit-source-id: 3fa9b8669bf3bf7617bfc05ef8f23d52bc969b4e
Summary:
I was unsure how, or if, DecoratedHostObjects worked. Marc had created the linked task to make sure that things worked right with multiple levels of decorated host objects. I decided to do that, thinking it would show a bug; instead it showed me that the pattern did work, and let me figure out how.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D20263797
fbshipit-source-id: 8bbe71cba1b6ef5b2673566759112320dd1894b8
Summary:
We are moving towards 100%-prettified files. That's the first step when we apply Clang Format for `ReactCommon`.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20110895
fbshipit-source-id: 0a0ce4997cf1c3721b0b07ef78c1a57ce87d20f9
Summary: The `Function::call` method says it leaves the JS `this` object undefined. According to my tests, that's not completely true: if the function is defined to use strict mode via `"use strict"` either inside itself or in the file it was defined in, it does leave it `undefined`, but if the function is defined in non-strict mode, it sets `this` to the global object instead. This diff updates the documentation to reflect this.
Reviewed By: mhorowitz
Differential Revision: D19613483
fbshipit-source-id: 4b9ecf81c6318592be05a216748dcb22e32989f8
Summary:
We recently updated React Native's docs site to have its own domain reactnative.dev and needed to update the URLs in the source code
CHANGELOG:
[INTERNAL]
Reviewed By: hramos
Differential Revision: D20072842
fbshipit-source-id: 1970d9214c872a6e7abf697d99f8f5360b3b308e
Summary:
[A recent change to JSStringToSTLString](https://github.com/facebook/react-native/pull/26955) causes a crash when the function is invoked with invalid UTF-16 data. The old behaviour, restored here, was to truncate the string before the first invalid character.
Here's how [the original code](https://github.com/facebook/react-native/blob/aee88b6843cea63d6aa0b5879ad6ef9da4701846/ReactCommon/jsi/JSCRuntime.cpp#L287) handled this case:
```
std::string JSStringToSTLString(JSStringRef str) {
size_t maxBytes = JSStringGetMaximumUTF8CStringSize(str);
// ^ maxBytes >= 1 regardless of str's contents
std::vector<char> buffer(maxBytes);
// ^ vector is zero initialised
JSStringGetUTF8CString(str, buffer.data(), maxBytes);
// ^ writes '\0' at the first invalid character and returns early (see JSC source code)
return std::string(buffer.data());
// ^ copies the string up to the first '\0'
}
```
See the JSC implementations of [`JSStringGetUTF8CString`](https://opensource.apple.com/source/JavaScriptCore/JavaScriptCore-7600.8.7/API/JSStringRef.cpp.auto.html) and [`convertUTF16ToUTF8`](https://opensource.apple.com/source/WTF/WTF-7600.7.2/wtf/unicode/UTF8.cpp.auto.html).
Based on the fact that `JSStringGetUTF8CString` *always* null-terminates the buffer - even when it bails out of converting an invalid string - here we're able to both
1. keep the fast path (not zero-initialising, not scanning for the null terminator) for the common case when the data is valid and JSStringGetUTF8CString returns a nonzero length; and
2. return the truncated string when JSStringGetUTF8CString returns an error code of 0, by scanning for the null terminator.
Changelog: [General] [Fixed] - Fix crash when passing invalid UTF-16 data from JSC into native code
Differential Revision: D19902751
fbshipit-source-id: 06bace2719800e921ec115ad6a29251eafd473f6
Summary:
In all other areas of JSI, std::string is treated as potentially
containing UTF-8 bytes (instead of ASCII). This fixes the inconsistency.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D19871520
fbshipit-source-id: c703f07e10bedbf2518d0bec903f85f43bbcbdf5
Summary:
If we're using synth traces in production, we probably don't want to accumulate the trace in memory, and write it to a file in one big step when an error happens. Rather, we'd like to write the trace incrementally, to a file: file system space is less scarse than memory, and doing it incrementally means less to do in the error handler.
This diff is the first step towards writing the synth trace incrementally. The most difficult aspect of this is that in the existing code, on Android, the trace is written in the handler of an Intent. Java-level operations on the Intent yielded a temporary filename under the application (so if the application is uninstalled, these temporary files are deleted). We must do something similar, but choose the filename within native code, on construction of the SynthTrace object. We copy what profiling does for this: assume the tmp dir is /data/data/<app>, where <app> can be found from reading /proc/self/cmdLine.
The SynthTrace constructor now takes a new argument: a unique_pointer to a stream. If null, no trace file is written, and the SynthTrace just accumulates records in memory. (This functionality is used during trace replay.) . If non-null, the trace is written to that stream.
The "write{BridgeTraffic}Trace..." methods become "flushAndDisable{BridgeTrafficTrace}" methods.
This diff is the first step towards incremental traces: the trace is still written at the end, but we've arranged that the place to write it is available at construction of the SynthTrace object. Later diffs will move recording of static things (e.g., the RuntimeConfig) into the ctor, then actually make us write the trace to the file incrementally.
For ReactNative:
Changelog: [Internal]
Reviewed By: haozhun, dulinriley
Differential Revision: D19471297
fbshipit-source-id: c1de4d2d9f44a87c7ff6fea38a1ce67de593940c
Summary:
Instead of returning a `bool` which gives no information about the cause of the error,
return `void` and throw when there's some error.
Another alternative is returning `std::error_code`, but that's less flexible than throwing, and
this API already supports throwing.
Changelog: [Internal]
Reviewed By: jbower-fb
Differential Revision: D19170033
fbshipit-source-id: 870cd996a1a53c94524455f31765c1da99f57a1d
Summary:
The documentation in two out of the three variants of `Function::callWithThis` incorrectly stated that the `this` value was to be `undefined` instead of set to `jsThis`, which contradicts the point of the `callWithThis` method existing in the first place. This diff fixes that issue.
Changelog: [General] [Fixed] - Fix documentation comments for HermesJS's `Function::callWithThis` method to accurately reflect how `this` is handled.
Reviewed By: jbower-fb
Differential Revision: D19714074
fbshipit-source-id: 123688c1f7e578d6356bec54604fb6e30f06b0b1
Summary:
Adding a `.clang-tidy` to a bunch of dirs under `react-native-github/ReactAndroid` and `react-native-github/ReactCommon`.
I don't want to add a single `.clang-tidy` at the root because we'll need more fine-grained control over what checks are enabled in different parts of the codebase; for example, fabric will and TM will probably have more checks enabled than older parts of the codebase that we're not actively modernizing, and the Hermes team probably wants its own config to be consistent with the rest of their codebase.
Starting off each `.clang-tidy` by only enabling clang-diagnostic; this is just to test that it's working. In the future, we'll work with the community to gradually enable more checks.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19705749
fbshipit-source-id: 979cea053b645ac4a9790340033bfcfb49ca0f97
Summary:
In my app I have a case where I need to pass a very large string (45MB) between JS and native. This is obviously suboptimal, but… this is where I'm at.
The main bottleneck to doing this turned out to be `jsi`'s `JSStringToSTLString()`, which was extremely slow. In my case, 4.7s to execute. After this change, 204ms.
I don't really know C++, so I'm not sure this code is 100% correct and safe, and I bet it could be done even better by avoiding the extra memory allocation (would shave off another 70ms).
## Changelog
[General] [Changed] - Make JSStringToSTLString 23x faster
Pull Request resolved: https://github.com/facebook/react-native/pull/26955
Reviewed By: shergin
Differential Revision: D19578728
Pulled By: motiz88
fbshipit-source-id: 2fbce83166953ce928f0a6aa36eed710bfe05383
Summary:
This change will keep project consistency.
This change repeat the previous commit 582738bdc8.
That was created by sammy-SC and reviewed by shergin
That changed
* `ASSERT_TRUE` -> `EXPECT_TRUE`
* `ASSERT_NEAR` -> `EXPECT_NEAR`
* `ASSERT_EQ` -> `EXPECT_EQ`
That said
> 1. Replace ASSERT_* with EXPECT_*. Assert is a fatal assertion. Expect is non-fatal assertion. So if assert fails, tests do not continue and therefore provide less information.
>
> 2. Rename tests in `RawPropsTest.cpp` from `ShadowNodeTest` to `RawPropsTest`.
>
> Source: https://github.com/google/googletest/blob/master/googletest/docs/primer.md#basic-assertions
## Changelog
[CATEGORY] [TYPE] - Message
Pull Request resolved: https://github.com/facebook/react-native/pull/27850
Differential Revision: D19568014
Pulled By: shergin
fbshipit-source-id: 7c22cd1c7ec919675e834a060bd5e681d43a8baf
Summary:
The reason for this change is that it is the primary root that we want people to be using and the naming should reflect that.
#nocancel
build-break
overriding_review_checks_triggers_an_audit_and_retroactive_review
Changelog: [Internal]
Oncall Short Name: fbobjc_sheriff
Differential Revision: D19431128
fbshipit-source-id: c7208e20ed0f5f5eb6c2849428c09a6d4af9b6f3
Summary:
This fixes a crash caused by a `JSError` exception not being successfully caught in a different dynamic library from where it was thrown. Since the libraries were compiled with hidden symbols and loaded with `RTLD_LOCAL`, the exception typeinfo becomes unique to each module.
Reading on this subject:
https://gcc.gnu.org/wiki/Visibilityhttps://stackoverflow.com/questions/14268736/symbol-visibility-exceptions-runtime-error
Reviewed By: mhorowitz
Differential Revision: D19343161
fbshipit-source-id: 4eb3bc2576bbcca2c3aef4f52b5a27dfde214c6a
Summary:
It is time to target SDK version 10.0+.
Changelog: [iOS] [Deprecated] - Deprecating support for iOS/tvOS SDK 9.x, 10.0+ is now required
Reviewed By: mdvacca
Differential Revision: D19265731
fbshipit-source-id: 93b6f9e8f61c5b36ff69e80d3f18256aa96cc2c0
Summary:
If building a JSError causes a JSError, this would lead to infinite recursion. This changes the error handling path so the problem is visible to JS. There's also a few related cleanups included, and a new test case.
Changelog: [General] [Fixed] - If Error global is not callable when building an error, jsi will throw a JS exception back to JS code. #158
Reviewed By: tmikov, dulinriley
Differential Revision: D18796269
fbshipit-source-id: 57a45d144fa2ea5e78d18c27d3130611737dda96
Summary:
This parameter used to be useful for a custom format hermes was developing,
but since Hermes now outputs the Chrome format it isn't useful.
Chrome actually disallows prettified JSON, and requires a special version that is faster to parse.
Therefore, `compact` was the only supported mode.
Changelog: [Internal] Remove compact parameter from `createSnapshotToFile`
Reviewed By: willholen
Differential Revision: D17726742
fbshipit-source-id: 6f39af9046dff2f3b4fba822312a9a89c939ed89
Summary:
`xplat` targets add different deps based on what platform the target is being built for.
for anything using `fb_xplat`, we can put all ios supermodules in `fbobjc_labels` and all android sms in `fbandroid_labels`
There's some weirdness with python targets like `thrift_gen` in `/xplat/mobileconfig/tools/generator/gen-py/BUCK` that don't have platform-specific labels because the except_for list for `fbandroid` doesn't need the `fbsource//` prefix (see changes in `/ios/isolation/infra.mobileconfig.sm`)
Changelog: [Internal]
Reviewed By: shergin, joshleibsly
Differential Revision: D17884952
fbshipit-source-id: e245364cf515b75682990094d24f789d53b1f3f5
Summary: Make it possible for a Runtime implementation to provide its own JSON parsing, by making the method on Value call through Runtime, which now has the default implementation.
Reviewed By: tmikov
Differential Revision: D17637395
fbshipit-source-id: b8997f7d1721a7790326417f3abfa86c875923c9
Summary:
Merging react-native-windows to 0.60 - the visual studio compiler seems to take issue with the existing code
## Changelog
[Internal] [Fixed] - Build fix for react-native-windows (MSVC)
Pull Request resolved: https://github.com/facebook/react-native/pull/26462
Test Plan:
No real change, just making compilers happy.
### Side Note
We'll want this change cherry-pickered to RN 0.60 and RN 0.61 so users of react-native-windows dont have to use our fork of react-native.
Reviewed By: mhorowitz
Differential Revision: D17406081
Pulled By: TheSavior
fbshipit-source-id: bc056e1a545c6478fdcbd5645f3a8dea657162c8
Summary:
`getHeapInfo` was creating and return a `jsi::Object`. This object was not being traced
when API tracing was turned on.
This resulted in a bug in synth traces, an object was being acted upon without its creation source
being logged.
In order to prevent this, change the API of `getHeapInfo` to no longer return any objects, or modify other heap
contents.
This is preferable to having `TracingRuntime` attempt to mimic the operations of two different implementations.
## Changelog:
[Internal] [Changed] - Changed `jsi::Instrumentation::getHeapInfo` to use a `std::unordered_map`
Reviewed By: mhorowitz
Differential Revision: D17273235
fbshipit-source-id: f69860dcc524c2cf501746a41dbac20b4db8c456
Summary: Include <cstdarg>, which defines the va_start and va_end macros.
Reviewed By: tmikov
Differential Revision: D17344433
fbshipit-source-id: f51fb53cc6e016eeef1a9270c1f2c468a5edc967
Summary:
As part of the fix for https://github.com/facebook/react-native/issues/25349 I added `s.static_framework = true` to each podspec in repo (see https://github.com/facebook/react-native/pull/25619#discussion_r306993309 for more context).
This was required to ensure the existing conditional compilation with `#if RCT_DEV` and `__has_include` still worked correctly when `use_frameworks!` is enabled.
However, fkgozali pointed out that it would be ideal if we didn't have this requirement as it could make life difficult for third-party libraries.
This removes the requirement by moving `React-DevSupport.podspec` and `React-RCTWebSocket.podspec` into `React-Core.podspec` as subspecs. This means the symbols are present when `React-Core.podspec` is built dynamically so `s.static_framework = true` isn't required.
This means that any `Podfile` that refers to `React-DevSupport` or `React-RCTWebSocket` will need to be updated to avoid errors.
## Changelog
I don't think this needs a changelog entry since its just a refinement of https://github.com/facebook/react-native/pull/25619.
Pull Request resolved: https://github.com/facebook/react-native/pull/25816
Test Plan:
Check `RNTesterPods` still works both with and without `use_frameworks!`:
1. Go to the `RNTester` directory and run `pod install`.
2. Run the tests in `RNTesterPods.xcworkspace` to see that everything still works fine.
3. Uncomment the `use_frameworks!` line at the top of `RNTester/Podfile` and run `pod install` again.
4. Run the tests again and see that it still works with frameworks enabled.
Reviewed By: hramos
Differential Revision: D16495030
Pulled By: fkgozali
fbshipit-source-id: 2708ac9fd20cd04cb0aea61b2e8ab0d931dfb6d5
Summary:
This is my proposal for fixing `use_frameworks!` compatibility without breaking all `<React/*>` imports I outlined in https://github.com/facebook/react-native/pull/25393#issuecomment-508457700. If accepted, it will fix https://github.com/facebook/react-native/issues/25349.
It builds on the changes I made in https://github.com/facebook/react-native/pull/25496 by ensuring each podspec has a unique value for `header_dir` so that framework imports do not conflict. Every podspec which should be included in the `<React/*>` namespace now includes it's headers from `React-Core.podspec`.
The following pods can still be imported with `<React/*>` and so should not have breaking changes: `React-ART`,`React-DevSupport`, `React-CoreModules`, `React-RCTActionSheet`, `React-RCTAnimation`, `React-RCTBlob`, `React-RCTImage`, `React-RCTLinking`, `React-RCTNetwork`, `React-RCTPushNotification`, `React-RCTSettings`, `React-RCTText`, `React-RCTSettings`, `React-RCTVibration`, `React-RCTWebSocket` .
There are still a few breaking changes which I hope will be acceptable:
- `React-Core.podspec` has been moved to the root of the project. Any `Podfile` that references it will need to update the path.
- ~~`React-turbomodule-core`'s headers now live under `<turbomodule/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- ~~`React-turbomodulesamples`'s headers now live under `<turbomodulesamples/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- ~~`React-TypeSaferty`'s headers now live under `<TypeSafety/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511040967.
- ~~`React-jscallinvoker`'s headers now live under `<jscallinvoker/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- Each podspec now uses `s.static_framework = true`. This means that a minimum of CocoaPods 1.5 ([released in April 2018](http://blog.cocoapods.org/CocoaPods-1.5.0/)) is now required. This is needed so that the ` __has_include` conditions can still work when frameworks are enabled.
Still to do:
- ~~Including `React-turbomodule-core` with `use_frameworks!` enabled causes the C++ import failures we saw in https://github.com/facebook/react-native/issues/25349. I'm sure it will be possible to fix this but I need to dig deeper (perhaps a custom modulemap would be needed).~~ Addressed by https://github.com/facebook/react-native/pull/25619/commits/33573511f02f3502a28bad48e085e9a4b8608302.
- I haven't got Fabric working yet. I wonder if it would be acceptable to move Fabric out of the `<React/*>` namespace since it is new? �
## Changelog
[iOS] [Fixed] - Fixed compatibility with CocoaPods frameworks.
Pull Request resolved: https://github.com/facebook/react-native/pull/25619
Test Plan:
### FB
```
buck build catalyst
```
### Sample Project
Everything should work exactly as before, where `use_frameworks!` is not in `Podfile`s. I have a branch on my [sample project](https://github.com/jtreanor/react-native-cocoapods-frameworks) here which has `use_frameworks!` in its `Podfile` to demonstrate this is fixed.
You can see that it works with these steps:
1. `git clone git@github.com:jtreanor/react-native-cocoapods-frameworks.git`
2. `git checkout fix-frameworks-subspecs`
3. `cd ios && pod install`
4. `cd .. && react-native run-ios`
The sample app will build and run successfully. To see that it still works without frameworks, remove `use_frameworks!` from the `Podfile` and do steps 3 and 4 again.
### RNTesterPods
`RNTesterPodsPods` can now work with or without `use_frameworks!`.
1. Go to the `RNTester` directory and run `pod install`.
2. Run the tests in `RNTesterPods.xcworkspace` to see that everything still works fine.
3. Uncomment the `use_frameworks!` line at the top of `RNTester/Podfile` and run `pod install` again.
4. Run the tests again and see that it still works with frameworks enabled.
Reviewed By: PeteTheHeat
Differential Revision: D16465247
Pulled By: PeteTheHeat
fbshipit-source-id: cad837e9cced06d30cc5b372af1c65c7780b9e7a
Summary:
Based on Hermes Issue: https://github.com/facebook/hermes/issues/47
It was not actually a bug in Hermes, but a bug in JSI, assuming that all string property names from
`folly::dynamic` are ASCII.
Now we'll be more intentional and directly state `forUtf8` rather than the implicit ASCII encoding.
Reviewed By: mhorowitz
Differential Revision: D16347857
fbshipit-source-id: 6bcfbf9f918dc0a7a485b88a1b537d6c2dd322cc
Summary: Ran clang-format on all of JSI.
Reviewed By: mhorowitz
Differential Revision: D16347858
fbshipit-source-id: 004afde1944f60a2c0989a7c38d5b3c58587f1cb
Summary:
This is the first step towards fixing https://github.com/facebook/react-native/issues/25349. These are the changes to the podspec to correctly update dependencies and build config that will cause any breaking change for users or libraries.
I am breaking these changes out from https://github.com/facebook/react-native/pull/25393 as suggested by fkgozali in https://github.com/facebook/react-native/pull/25393#issuecomment-508322884.
These are the changes:
- Made C++ headers in `React-Core` private by default so that ObjC files can import the module without failures.
- Reduced the number of `yoga` headers that are exposed for the same reason as above. As far as I can see this doesn't cause issues but we can find another solution if it does.
- Adding some missing dependencies to fix undefined symbols errors.
- Added `DoubleConversion` to `HEADER_SEARCH_PATHS` where it was missing.
## Changelog
[iOS] [Fixed] - Updated podspecs for improved compatibility with different install types.
Pull Request resolved: https://github.com/facebook/react-native/pull/25496
Test Plan:
Everything should work exactly as before. I have a branch on my [sample project](https://github.com/jtreanor/react-native-cocoapods-frameworks) here which points at this branch to show that it is still working `Podfile` to demonstrate this is fixed.
You can see that it works with these steps:
1. `git clone git@github.com:jtreanor/react-native-cocoapods-frameworks.git`
2. `git checkout podspec-updates`
3. `cd ios && pod install`
4. `cd .. && react-native run-ios`
The sample app will build and run successfully.
Reviewed By: mmmulani
Differential Revision: D16167346
Pulled By: fkgozali
fbshipit-source-id: 1917b2f8779cb172362a457fb3fce686c55056d3
Summary:
The instrumentation header only needs the forward declarations for ostream, so
we can use just include `iosfwd`, as suggested by Riley in an earlier diff.
Reviewed By: kodafb
Differential Revision: D16152451
fbshipit-source-id: 2afbc40e623b180dfc5917fc8093ab15bf647968
Summary:
Adding an instrumentation endpoint that allows us to write a snapshot out to an
arbitrary output stream. This is in addition to `createSnapshotFromFile`. I
reserve the right to replace the latter with the former in a later diff.
This is necessary to allow snapshots to be requested over the chrome debugger
protocol. The protocol sends the snapshot over the wire in chunks so we need
to be able to use an output stream that can do the chunking.
Because LLVM types are not available at the JSI layer, we accept a
`std::ostream` reference which we wrap with `llvm::raw_os_ostream` within
Hermes. We should not incur the static initializer cost (or other code bloat)
of using ostreams as a result.
Reviewed By: cwdick
Differential Revision: D16016319
fbshipit-source-id: 2d0b4848fd5cbe9ddee371d856cd8eb19dd80396
Summary:
The function is not annotated with `const` so `plain()` will return a non-const
reference to the undecorated Runtime already. Seems like the const_cast was a
hold-over from a previous iteration.
Reviewed By: mhorowitz
Differential Revision: D16016320
fbshipit-source-id: 3dfa1e9acf2fc5c1ad61c9a8cd27c3c2e42036d3
Ryan Tremblay
Marc Horowitz
Jacob Bower
Kevin Gozali
David Detlefs
Valentin Shergin
Gabriel Nunes
Rachel Nabors
Radek Pietruszewski
Janette Cheng
Moti Zilberman
Haozhun Jin
Riley Dulin
Josh Leibsly
Emily Janzer
Yuki Ueda
Kudo Chien
Scott Kyle
Andres Suarez
Adam Ernst
Daniel Andersson
REDMOND\acoates
SachinTotale
James Treanor
Min ho Kim
Ashok Menon
