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

Event name normalization for Fabric interop #42586

Closed
wants to merge 1 commit into from

Conversation

dmytrorykun
Copy link
Contributor

@dmytrorykun dmytrorykun commented Jan 22, 2024

Summary:
Every event name must be normalized.

The normalization strategy is:

  1. If it starts with top -> do nothing.
  2. If it starts with on -> replace on with top.
  3. Else -> capitalize the first character and prepend top.

We have it for the old renderer on iOS here. This one is also used by the interop layer.
Static ViewConfigs being a part of the new renderer replicate this behavior to match the old rendered.

The Android that we have is incomplete, it is missing the [2. If it starts with on -> replace on with top]. This means that some events names that worked with the old renderer would not be compatible with the new renderer + Static ViewConfigs. Specifically every event names that start with on.

This diff implements event name normalization on Android.

Changelog: [Internal] - Update event normalization algorithm to match SVCs.

Differential Revision: D50604571

Summary:
Every event name must be normalized.

The normalization strategy is:
1. If it starts with `top` -> do nothing.
2. If it starts with `on` -> replace `on` with `top`.
3. Else -> capitalize the first character and prepend `top`.

We have it for the old renderer on iOS [here](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native/React/Base/RCTEventDispatcher.m#L12-L21). This one is also used by the interop layer.
Static ViewConfigs being a part of the new renderer [replicate](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js#L164-L172) this behavior to match the old rendered.

We've never had proper implementation for this on Android. So some events names that worked with the old renderer would not be compatible with the new renderer + Static ViewConfigs. Specifically every event name now must start with `top`.

This diff implements event name normalization on Android.

Changelog: [Internal] - Update event normalization algorithm to match SVCs.

Differential Revision: D50604571
@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 labels Jan 22, 2024
@facebook-github-bot
Copy link
Contributor

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

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,869,128 -25
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,252,921 -10
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 5e8ce6c
Branch: main

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 29, 2024
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in b32c6c2.

lunaleaps pushed a commit that referenced this pull request Feb 4, 2024
Summary:
Pull Request resolved: #42586

Every event name must be normalized.

The normalization strategy is:
1. If it starts with `top` -> do nothing.
2. If it starts with `on` -> replace `on` with `top`.
3. Else -> capitalize the first character and prepend `top`.

We have it for the old renderer on iOS [here](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native/React/Base/RCTEventDispatcher.m#L12-L21). This one is also used by the interop layer.
Static ViewConfigs being a part of the new renderer [replicate](https://github.com/facebook/react-native/blob/a7586947d719a9cd2344ad346d271e7ca900de87/packages/react-native-codegen/src/generators/components/GenerateViewConfigJs.js#L164-L172) this behavior to match the old rendered.

The Android that we have is incomplete, it is missing the [*2. If it starts with `on` -> replace `on` with `top`*]. This means that some events names that worked with the old renderer would not be compatible with the new renderer + Static ViewConfigs. Specifically every event names that start with `on`.

This diff implements event name normalization on Android.

Changelog: [Internal] - Update event normalization algorithm to match SVCs.

Reviewed By: cortinico

Differential Revision: D50604571

fbshipit-source-id: cef34d8baa2cf31f641be423a16bca7ea9fa20c4
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