Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make enums to work as part of data structures for C++ TurboModules codegen #36155

Closed
wants to merge 4 commits into from

Conversation

rshest
Copy link
Contributor

@rshest rshest commented Feb 14, 2023

Summary:
[Changelog][Internal]

The PR #36030 (diff D42884147 (ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (963e45a), but there are also some changes related to the types, that were required.

Differential Revision: D43292254

rshest and others added 4 commits February 14, 2023 15:49
…ok#36116)

Summary:
Pull Request resolved: facebook#36116

[Changelog][Internal]

Add a minimal/reference JavaScript implementation for NativePerformanceObserver - the purpose is both unit testing (JS and native sides separately) and potentially shimming the part of functionality that is not dependent on native side.

This is both a setup for adding general unit tests for the Performance* APIs, but also to be able to do non-trivial changes on JS side for WebPerformance (such as in (D43154319).

Differential Revision: https://internalfb.com/D43167392

fbshipit-source-id: 5050042d4923a87c87d4e8e99ba1ef6b3f264649
Summary:
[Changelog][Internal]

By [the W3C standard](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceObserver/observe), `PerformanceObserver.observer` can optionally take a `durationThreshold` option, so that only entries with duration larger than the threshold are reported.

This diff adds support for this on the RN side.

NOTE: The standard suggests that default value for this is 104s. I left it at 0 for now, as for the RN use cases t may be to too high (needs discussion).

Differential Revision: https://internalfb.com/D43154319

fbshipit-source-id: 884ca22b62daf241d1b98facc58958dab1797633
Summary:
[Changelog][Internal]

Implements EventCounts API (`Performance.eventCounts`) for Web Performance, according to the W3C standard: https://www.w3.org/TR/event-timing/#eventcounts

Differential Revision: https://internalfb.com/D43285073

fbshipit-source-id: e86b719305201fc7be7f6de9b65a1d321d58d140
…degen

Summary:
[Changelog][Internal]

The PR  facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required.

Differential Revision: D43292254

fbshipit-source-id: 50066dc89c495ba3cad23a26024ac485d03dc9d9
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Feb 14, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D43292254


import NativePerformanceObserver from './NativePerformanceObserver';
import {warnNoNativePerformanceObserver} from './PerformanceObserver';
import {rawToPerformanceEntryType} from './RawPerformanceEntry';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • ⚠️ Libraries/WebPerformance/EventCounts.js line 13 – 'rawToPerformanceEntryType' is defined but never used. (no-unused-vars)

@github-actions
Copy link

Fails
🚫

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against 36b0228

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,485,994 +421
android hermes armeabi-v7a 7,806,757 +395
android hermes x86 8,963,167 +360
android hermes x86_64 8,820,255 +464
android jsc arm64-v8a 6,699,708 +419
android jsc armeabi-v7a 7,491,190 +397
android jsc x86 9,224,280 +355
android jsc x86_64 6,925,015 +487

Base commit: 11570e7
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Feb 15, 2023
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 96fb708.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
…degen (facebook#36155)

Summary:
Pull Request resolved: facebook#36155

[Changelog][Internal]

The PR  facebook#36030 (diff D42884147 (facebook@ceb1d0d)) added support for enum types in JS to C++ bridging in C++ TurboModules.

This only worked for enums as argument types for exposed methods, but not for the cases when enums are members of complex data structures that are also exposed through a codegen.

This diff fixes this problem, so that codegen now correctly works both with enum types as method arguments, but also as data structure members.

Some part of the change is the same as D42008724 (facebook@963e45a), but there are also some changes related to the types, that were required.

Reviewed By: christophpurrer

Differential Revision: D43292254

fbshipit-source-id: b2d6cf4a2d4d233b8cc403ecd02b5be16d5d91a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants