Skip to content

Conversation

@NickGerleman
Copy link
Contributor

Summary:
ViewManager’s may implement a measure function (using MapBuffer, or ReadableMap). This isn’t used automatically, but may instead be used, by calling into FabricUIManager via JNI, and passing the component name of the view manager to use.

This is only ever called manually, through specific C++ ShadowNodes. Confusingly, for some cases, like TextInput, we use the measure function on RCTText instead, because this call to FabricUIManager is hidden behind TextLayoutManager. This ends up really breaking Facsimile, since we want to measure these in a different way, while still measuring TextInput without preparing a layout.

This mechanism is also used to inject ReactTextViewManager from the Text View Manager, into the measurement process, for both Text, and TextInput.

I think we would ideally remove and replace the current View Manager measurement mechanism entirely. The interface doesn’t do what it claims to, and requires calling private Java methods via JNI, which we shouldn’t be encouraging external libraries to do. Only a single 3p librar (react-native-picker) uses this, but we have a lot of internal usages, and the current facility is valuable, for translating surface ID into a context. Ie we could not deprecate it without a well thought out replacement.

Instead, this change:

  1. Removes the "generalized" version of this for MapBuffer, only ever used by Text
  2. Given ourselves a measureText function, that will use a spannable processor provided, but go through TextLayoutManager, instead of trying to use this generalized path
  3. Documents some of the weirdness of the current setup, without yet deprecating it

This will let the Facsimile View Manager:

  1. Provide a ReactTextViewManagerCallback, like the previous version allowed, that influences measurement of both Text, and TextInput (which is... strange, but... not trying to boil the ocean here)
  2. TextInput can now measure text, even if Facsimile View Manager doesn't implement this traditional measure interface

Changelog:
[Android][Breaking] - Clean up measurements path and ReactTextViewManagerCallback injection

Differential Revision: D75826792

@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 Jun 2, 2025
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Jun 3, 2025
…facebook#51758)

Summary:

ViewManager’s may implement a measure function (using MapBuffer, or ReadableMap). This isn’t used automatically, but may instead be used, by calling into FabricUIManager via JNI, and passing the component name of the view manager to use.

This is only ever called manually, through specific C++ ShadowNodes. Confusingly, for some cases, like `TextInput`, we use the measure function on `RCTText` instead, because this call to FabricUIManager is hidden behind `TextLayoutManager`. This ends up really breaking Facsimile, since we want to measure these in a different way, while still measuring `TextInput` without preparing a layout.

This mechanism is also used to inject `ReactTextViewManager` from the Text View Manager, into the measurement process, for both Text, and TextInput.

I think we would ideally remove and replace the current View Manager measurement mechanism entirely. The interface doesn’t do what it claims to, and requires calling private Java methods via JNI, which we shouldn’t be encouraging external libraries to do. Only a single 3p librar (react-native-picker) uses this, but we have a lot of internal usages, and the current facility is valuable, for translating surface ID into a context. Ie we could not deprecate it without a well thought out replacement.

Instead, this change:
1. Removes the "generalized" version of this for MapBuffer, only ever used by Text
2. Given ourselves a `measureText` function, that will use a spannable processor provided, but go through TextLayoutManager, instead of trying to use this generalized path
3. Documents some of the weirdness of the current setup, without yet deprecating it

This will let the Facsimile View Manager:
1. Provide a ReactTextViewManagerCallback, like the previous version allowed, that influences measurement of both Text, and TextInput (which is... strange, but... not trying to boil the ocean here)
2. TextInput can now measure text, even if Facsimile View Manager doesn't implement this traditional measure interface

Changelog:
[Android][Breaking] - Remove FabricUIManager.measureMapBuffer() and MapBuffer measure functions on ViewManager. Please use ReadableMap variant.
[Android][Breaking] - Remove FabricUIManager.measure overload which accepts attachment positions

Differential Revision: D75826792
…facebook#51758)

Summary:

ViewManager’s may implement a measure function (using MapBuffer, or ReadableMap). This isn’t used automatically, but may instead be used, by calling into FabricUIManager via JNI, and passing the component name of the view manager to use.

This is only ever called manually, through specific C++ ShadowNodes. Confusingly, for some cases, like `TextInput`, we use the measure function on `RCTText` instead, because this call to FabricUIManager is hidden behind `TextLayoutManager`. This ends up really breaking Facsimile, since we want to measure these in a different way, while still measuring `TextInput` without preparing a layout.

This mechanism is also used to inject `ReactTextViewManager` from the Text View Manager, into the measurement process, for both Text, and TextInput.

I think we would ideally remove and replace the current View Manager measurement mechanism entirely. The interface doesn’t do what it claims to, and requires calling private Java methods via JNI, which we shouldn’t be encouraging external libraries to do. Only a single 3p librar (react-native-picker) uses this, but we have a lot of internal usages, and the current facility is valuable, for translating surface ID into a context. Ie we could not deprecate it without a well thought out replacement.

Instead, this change:
1. Removes the "generalized" version of this for MapBuffer, only ever used by Text
2. Given ourselves a `measureText` function, that will use a spannable processor provided, but go through TextLayoutManager, instead of trying to use this generalized path
3. Documents some of the weirdness of the current setup, without yet deprecating it

This will let the Facsimile View Manager:
1. Provide a ReactTextViewManagerCallback, like the previous version allowed, that influences measurement of both Text, and TextInput (which is... strange, but... not trying to boil the ocean here)
2. TextInput can now measure text, even if Facsimile View Manager doesn't implement this traditional measure interface

Changelog:
[Android][Breaking] - Remove FabricUIManager.measureMapBuffer() and MapBuffer measure functions on ViewManager. Please use ReadableMap variant.
[Android][Breaking] - Remove FabricUIManager.measure overload which accepts attachment positions

Reviewed By: javache

Differential Revision: D75826792
@facebook-github-bot
Copy link
Contributor

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

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Jun 5, 2025
…facebook#51758)

Summary:

ViewManager’s may implement a measure function (using MapBuffer, or ReadableMap). This isn’t used automatically, but may instead be used, by calling into FabricUIManager via JNI, and passing the component name of the view manager to use.

This is only ever called manually, through specific C++ ShadowNodes. Confusingly, for some cases, like `TextInput`, we use the measure function on `RCTText` instead, because this call to FabricUIManager is hidden behind `TextLayoutManager`. This ends up really breaking Facsimile, since we want to measure these in a different way, while still measuring `TextInput` without preparing a layout.

This mechanism is also used to inject `ReactTextViewManager` from the Text View Manager, into the measurement process, for both Text, and TextInput.

I think we would ideally remove and replace the current View Manager measurement mechanism entirely. The interface doesn’t do what it claims to, and requires calling private Java methods via JNI, which we shouldn’t be encouraging external libraries to do. Only a single 3p librar (react-native-picker) uses this, but we have a lot of internal usages, and the current facility is valuable, for translating surface ID into a context. Ie we could not deprecate it without a well thought out replacement.

Instead, this change:
1. Removes the "generalized" version of this for MapBuffer, only ever used by Text
2. Given ourselves a `measureText` function, that will use a spannable processor provided, but go through TextLayoutManager, instead of trying to use this generalized path
3. Documents some of the weirdness of the current setup, without yet deprecating it

This will let the Facsimile View Manager:
1. Provide a ReactTextViewManagerCallback, like the previous version allowed, that influences measurement of both Text, and TextInput (which is... strange, but... not trying to boil the ocean here)
2. TextInput can now measure text, even if Facsimile View Manager doesn't implement this traditional measure interface

Changelog:
[Android][Breaking] - Remove FabricUIManager.measureMapBuffer() and MapBuffer measure functions on ViewManager. Please use ReadableMap variant.
[Android][Breaking] - Remove FabricUIManager.measure overload which accepts attachment positions

Reviewed By: javache

Differential Revision: D75826792
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 2ba86ca.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 5, 2025
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @NickGerleman in 2ba86ca

When will my fix make it into a release? | How to file a pick request?

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