Flush discrete passive effects before paint

[acdlite]

If a discrete render results in passive effects, we should flush them synchronously at the end of the current task so that the result is immediately observable. For example, if a passive effect adds an event listener, the listener will be added before the next input.

We don't need to do this for effects that don't have discrete/sync priority, because we assume they are not order-dependent and do not need to be observed by external systems.

For legacy mode, we will maintain the existing behavior, since it hasn't been reported as an issue, and we'd have to do additional work to distinguish "legacy default sync" from "discrete sync" to prevent all passive effects from being treated this way.

[sizebot]

Comparing: b48b38a...52a7680

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	+0.02%	122.62 kB	122.65 kB	+0.03%	39.34 kB	39.35 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.02%	129.19 kB	129.21 kB	+0.03%	41.42 kB	41.43 kB
facebook-www/ReactDOM-prod.classic.js	+0.02%	405.80 kB	405.89 kB	+0.02%	75.37 kB	75.39 kB
facebook-www/ReactDOM-prod.modern.js	+0.02%	394.06 kB	394.15 kB	+0.02%	73.51 kB	73.53 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.02%	405.80 kB	405.89 kB	+0.02%	75.37 kB	75.39 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 52a7680

[bvaughn]

FWIW, this change broke the settings modal dialog in DevTools. (Haven't dug into why yet, just found this commit by bisecting.)

[gaearon]

Probably similar to repro case in #20074 (comment) which this change made reproduce consistently.

[bvaughn]

Yeah, seems to be the same issue.

Looks like this click handler is firing and changing the context value to true:

react/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContextToggle.js

Lines 23 to 25 in b4f119c

	const showFilterModal = useCallback(() => setIsModalShowing(true), [
	setIsModalShowing,
	]);

And then, as part of the same "click" event, this handler is now also being called and dismissing the modal:

react/packages/react-devtools-shared/src/devtools/views/hooks.js

Lines 200 to 243 in b4f119c

	export function useModalDismissSignal(
	modalRef: {current: HTMLDivElement | null, ...},
	dismissCallback: () => void,
	dismissOnClickOutside?: boolean = true,
	): void {
	useEffect(() => {
	if (modalRef.current === null) {
	return () => {};
	}
	const handleDocumentKeyDown = ({key}: any) => {
	if (key === 'Escape') {
	dismissCallback();
	}
	};
	const handleDocumentClick = (event: any) => {
	// $FlowFixMe
	if (
	modalRef.current !== null &&
	!modalRef.current.contains(event.target)
	) {
	event.stopPropagation();
	event.preventDefault();
	dismissCallback();
	}
	};
	// It's important to listen to the ownerDocument to support the browser extension.
	// Here we use portals to render individual tabs (e.g. Profiler),
	// and the root document might belong to a different window.
	const ownerDocument = modalRef.current.ownerDocument;
	ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
	if (dismissOnClickOutside) {
	ownerDocument.addEventListener('click', handleDocumentClick);
	}
	return () => {
	ownerDocument.removeEventListener('keydown', handleDocumentKeyDown);
	ownerDocument.removeEventListener('click', handleDocumentClick);
	};
	}, [modalRef, dismissCallback, dismissOnClickOutside]);
	}

Obviously I can fix this on the DevTools side 😄

Fixed via #21173