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

react-native/packages/react-native/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm

Lines 75 to 78 in 06eea61

	NSDictionary *userInfo = [NSDictionary dictionaryWithObjectsAndKeys:scrollEvent, @"event", nil];
	[[NSNotificationCenter defaultCenter] postNotificationName:@"RCTNotifyEventDispatcherObserversOfEvent_DEPRECATED"
	object:nil
	userInfo:userInfo];

but without using currentBridge since it is considered bad practice: software-mansion/react-native-reanimated#5497 (comment).

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:

bridgelessSticky.mov

• bridge:

bridgeSticky.mov

• old arch:

oldarchSticky.mov

[cipolleschi]

Looks good! I know it is a bit of a work, but could you add in the test plan a video or something showing that the event is working in all the use cases:

• Old Architecture
• New Architecture (with Bridge - for the internal use case)
• New Architecture (with bridgeless)

Thank you so much!

[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: 93c079b
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 f5c888c.

[github-actions]

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

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