feat: move notifying observers to event dispatcher

[WoLewicki]

Summary:

Based on the discussion starting here: https://discord.com/channels/514829729862516747/1073566663825432587/1237407161991172157, I suggest moving the call to _notifyEventDispatcherObserversOfEvent_DEPRECATED straight to RCTEventDispatcher.mm. It was previously in RCTInstance.mm which is only relevant on bridgeless mode. We want to mimic the behavior of https://github.com/facebook/react-native/blob/06eea61c19cd730cf0c14a436f042d30791c3f4a/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm#L75-L78 but without using currentBridge since it is considered bad practice: https://github.com/software-mansion/react-native-reanimated/issues/5497#issuecomment-2083400038.

Changelog:

[IOS] [CHANGED] - Move notifyObservers straight to RCTEventDispatcher.mm.

Test Plan:

See that example with stickyHeaders still works correctly on both bridgeless and bridge mode.

Videos with it on https://github.com/facebook/react-native/blob/deee037c62a7d62a349d34db427b14d3560ddf83/packages/rn-tester/js/examples/FlatList/FlatList-stickyHeaders.js example with more items for visibility:

• bridgeless:

https://github.com/facebook/react-native/assets/32481228/8b78104a-226b-466a-9f32-60ba4ec14100

• bridge:

https://github.com/facebook/react-native/assets/32481228/f2ca67cb-578f-45d4-954f-3249c6fa9410

• old arch:

https://github.com/facebook/react-native/assets/32481228/7d642923-ddda-4dd3-8f14-c9982a03bc2e

[WoLewicki]

Done ✅

[WoLewicki]

One downside I see about it is that we now add the listener on old arch too where it is not used. But it does not seem too problematic I guess?

[cipolleschi]

One downside I see about it is that we now add the listener on old arch too where it is not used. But it does not seem too problematic I guess?

Yeah, if nothing in the Old Architecture fires the RCTNotifyEventDispatcherObserversOfEvent_DEPRECATED event, that's not an issue!

Out of curiosity, have you tried to remove the pestering code here? Does ScrollView still works properly without it?

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,542,779	+26
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,912,791	-26
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 93c079b92a028354cb7926b2029601238bf82d7d Branch: main

[WoLewicki]

Out of curiosity, have you tried to remove the pestering code here?

Forgot about this one, trying it now.

[WoLewicki]

Yeah, it works just fine when this line is removed 🚀 I'll remove it then!

[cipolleschi]

/rebase - this comment automatically rebase on top of main

[cipolleschi]

Finally back to this! :D

[facebook-github-bot]

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[cipolleschi]

/rebase - this comment automatically rebase on top of main

[cipolleschi]

@WoLewicki I need to work a little on this as I cannot land it as it is, due to how the internal system works... is it okay for you?

[WoLewicki]

@cipolleschi yeah no problem, as long as we fix the issue 🚀

[facebook-github-bot]

@cipolleschi merged this pull request in facebook/react-native@f5c888c2d7c6dfe009de5df2cc795aff26286c19.

[github-actions[bot]]

This pull request was successfully merged by @WoLewicki in f5c888c2d7c6dfe009de5df2cc795aff26286c19.

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