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

POC: Transition compatibility hooks #44151

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions packages/mui-material/src/Autocomplete/Autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { useDefaultProps } from '../DefaultPropsProvider';
import autocompleteClasses, { getAutocompleteUtilityClass } from './autocompleteClasses';
import capitalize from '../utils/capitalize';
import useSlot from '../utils/useSlot';
import { useDeferredValue } from '@mui/utils';
import Fade from '../Fade';
Copy link
Member

@oliviertassinari oliviertassinari Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be inconsistent with the Select animation, no?

Suggested change
import Fade from '../Fade';

I guess it's also outside of the scope of the problem we go after?


const useUtilityClasses = (ownerState) => {
const {
Expand Down Expand Up @@ -422,6 +424,7 @@ const AutocompleteGroupUl = styled('ul', {

export { createFilterOptions };

const EMPTY_ARRAY = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really wish the spec would add something like Array.EMPTY, we keep adding empty values like this all over the place.

Copy link
Member

@Janpot Janpot Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we keep adding empty values like this all over the place.

We could add it in our utils package? Not sure there would be a noticeable benefit though 🙂.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure there would be a noticeable benefit though

It avoids useless memory allocations, and in some situations it does make an impact. A lot of the perf PRs I did in the last months were just removing memory allocations.

We could add it in our utils package

I usually do that, and it's frequent in other codebases as well, e.g. https://github.com/styled-components/styled-components/blob/main/packages/styled-components/src/utils/empties.ts

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure you'll notice a difference avoiding allocations in frequently called code. but we're talking about a single allocation at module scope in a few modules vs. allocating once and importing it. The code a bundler generates to import it from a utils package is probably more overhead than just allocating it in the module itself. I wouldn't be surprised if that styled-components snippet you share is actually slower than what we have here. Your proposal to make it part of the language makes a lot more sense to me.

Copy link
Contributor

@romgrk romgrk Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code a bundler generates to import it from a utils package is probably more overhead than just allocating it in the module itself.

Depends on the bundler, some of them generate a single scope where they just give unique names to all variables including imports/exports so "importing" something in prod is basically just using a variable (IIRC, vite does it), while others do a require-like snippet of code to replace imports (IIRC, webpack).

const Autocomplete = React.forwardRef(function Autocomplete(inProps, ref) {
const props = useDefaultProps({ props: inProps, name: 'MuiAutocomplete' });

Expand Down Expand Up @@ -572,6 +575,9 @@ const Autocomplete = React.forwardRef(function Autocomplete(inProps, ref) {
className: classes.paper,
});

const groupedOptionsDeferred = useDeferredValue(popupOpen ? groupedOptions : EMPTY_ARRAY);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in the hook version, no? So "Base UI" users can have the same benefit

Suggested change
const groupedOptionsDeferred = useDeferredValue(popupOpen ? groupedOptions : EMPTY_ARRAY);

const isLoading = loading || groupedOptionsDeferred !== groupedOptions;

const [PopperSlot, popperProps] = useSlot('popper', {
elementType: Popper,
externalForwardedProps,
Expand Down Expand Up @@ -672,19 +678,25 @@ const Autocomplete = React.forwardRef(function Autocomplete(inProps, ref) {

const renderAutocompletePopperChildren = (children) => (
<AutocompletePopper as={PopperSlot} {...popperProps}>
<AutocompletePaper as={PaperSlot} {...paperProps}>
{children}
</AutocompletePaper>
<Fade
in={popupOpen}
timeout={{ appear: 0, enter: isLoading ? 200 : 0, exit: 0 }}
easing="step-end"
>
<AutocompletePaper as={PaperSlot} {...paperProps}>
{children}
</AutocompletePaper>
</Fade>
</AutocompletePopper>
);

let autocompletePopper = null;
if (groupedOptions.length > 0) {
if (groupedOptionsDeferred.length > 0) {
autocompletePopper = renderAutocompletePopperChildren(
// TODO v7: remove `as` prop and move ListboxComponentProp to externalForwardedProps or remove ListboxComponentProp entirely
// https://github.com/mui/material-ui/pull/43994#issuecomment-2401945800
<ListboxSlot as={ListboxComponentProp} {...listboxProps}>
{groupedOptions.map((option, index) => {
{groupedOptionsDeferred.map((option, index) => {
if (groupBy) {
return renderGroup({
key: option.key,
Expand All @@ -698,13 +710,13 @@ const Autocomplete = React.forwardRef(function Autocomplete(inProps, ref) {
})}
</ListboxSlot>,
);
} else if (loading && groupedOptions.length === 0) {
} else if (isLoading && groupedOptionsDeferred.length === 0) {
autocompletePopper = renderAutocompletePopperChildren(
<AutocompleteLoading className={classes.loading} ownerState={ownerState}>
{loadingText}
</AutocompleteLoading>,
);
} else if (groupedOptions.length === 0 && !freeSolo && !loading) {
} else if (groupedOptionsDeferred.length === 0 && !freeSolo && !isLoading) {
autocompletePopper = renderAutocompletePopperChildren(
<AutocompleteNoOptions
className={classes.noOptions}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import * as ReactDOM from 'react-dom';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clean implementations 👌

import * as React from 'react';

export type Callback = () => void;
export type BatchCallback = (batch: Batch) => void;

interface Batch {
queue: Set<Callback>;
}

let ACTIVE_BATCH: Batch | null = null;
let PENDING_FLUSH: number = 0;
const PENDING_BATCHES: Set<Batch> = new Set();

/**
* Executes the given function inside of a batch.
*
* If a batch doesn't already exist, a new one will be created, and the given
* callback will be executed when it ends.
*/
export function runWithBatch<T>(fn: () => T, batchCallback: BatchCallback): T {
Copy link
Member

@oliviertassinari oliviertassinari Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we make transitions opt-in? Something that only React 19 users can adopt, and hence have no ponyfill?

Suggested change
export function runWithBatch<T>(fn: () => T, batchCallback: BatchCallback): T {

return ReactDOM.unstable_batchedUpdates(() => {
const prevBatch = ACTIVE_BATCH;
const batch = prevBatch == null ? ({ queue: new Set() } as Batch) : prevBatch;
let result: T;

try {
ACTIVE_BATCH = batch;
result = fn();
} finally {
ACTIVE_BATCH = prevBatch;
}

if (batch !== prevBatch) {
batchCallback(batch);
}
return result;
});
}

/**
* A batch callback that immediately executes all of the updates
* in every batch (the current one, and any pending). Assumes it's
* called in a ReactDOM batch.
*/
export function blockingBatchCallback(batch: Batch) {
flushPendingBatches();
batch.queue.forEach((callback) => callback());
}

/**
* A batch callback that executes every update in a future macro
* task. Assumes it's called in a ReactDOM batch.
*/
export function nonBlockingBatchCallback(batch: Batch) {
// Apply the pending batches with a timeout so that they
// are executed in a future macrotask, *after* the blocking
// changes have been painted.
//
// The timeout is a bit arbitrary. One of benefits of transitions are
// that they enable a kind of debouncing. With a non-zero timeout, we can
// get some of that benefit in React 17 by allowing non-blocking updates
// from e.g. keystrokes to cancel our previous timeout and further delay
// our deferred work instead of blocking the UI, with the trade-off of an
// increased latency to when the deferred work will be shown.
//
// The value should be something high enough that e.g. actively typing into
// a search box remains responsive, but not so high that the application
// feels slow to respond when you stop typing.
PENDING_BATCHES.add(batch);
window.clearTimeout(PENDING_FLUSH);
PENDING_FLUSH = window.setTimeout(() => {
ReactDOM.unstable_batchedUpdates(flushPendingBatches);
}, 375);
}

/**
* Creates a batch callback that executes every update in the given
* `startTransition` function.
*/
export function createPassthroughBatchCallback(startTransition: (callback: Callback) => void) {
return (batch: Batch) => {
startTransition(() => {
batch.queue.forEach((callback) => callback());
});
};
}

/**
* Attempt to enqueue the given state update.
*
* If there is an existing batch, the update will be added to it and
* run later. Otherwise, it will be run immediately, without batching.
*/
export function enqueueStateUpdate<T>(fn: Callback): Callback {
const queue = ACTIVE_BATCH?.queue;
if (queue) {
queue.add(fn);
return () => {
queue.delete(fn);
};
} else {
fn();
return () => {};
}
}

/**
* Flush any pending batches. Assumes it's called within a ReactDOM batch.
*/
function flushPendingBatches() {
window.clearTimeout(PENDING_FLUSH);
PENDING_FLUSH = 0;

PENDING_BATCHES.forEach((batch) => {
batch.queue.forEach((callback) => callback());
});
PENDING_BATCHES.clear();
}
1 change: 1 addition & 0 deletions packages/mui-utils/src/CompatTransitionManager/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './CompatTransitionManager';
4 changes: 4 additions & 0 deletions packages/mui-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,8 @@ export { default as unstable_resolveComponentProps } from './resolveComponentPro
export { default as unstable_extractEventHandlers } from './extractEventHandlers';
export { default as unstable_getReactNodeRef } from './getReactNodeRef';
export { default as unstable_getReactElementRef } from './getReactElementRef';
export { default as useDeferredValue } from './useDeferredValue';
export { default as useStateWithTransitions } from './useStateWithTransitions';
export { default as useReducerWithTransitions } from './useReducerWithTransitions';
export { default as useTransition } from './useTransition';
export * from './types';
2 changes: 2 additions & 0 deletions packages/mui-utils/src/useDeferredValue/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use client';
export { default } from './useDeferredValue';
35 changes: 35 additions & 0 deletions packages/mui-utils/src/useDeferredValue/useDeferredValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import * as React from 'react';
import {
nonBlockingBatchCallback,
enqueueStateUpdate,
runWithBatch,
} from '../CompatTransitionManager';

function useDeferredValue17<T>(value: T): T {
// React 17 doesn't support concurrent rendering. We simulate the behavior
// by only updating to the current value when the previous one is committed.
const [currentValue, setCurrentValue] = React.useState(value);

React.useEffect(() => {
if (value !== currentValue) {
return runWithBatch(
() => enqueueStateUpdate(() => setCurrentValue(value)),
nonBlockingBatchCallback,
);
}
}, [value, currentValue]);

return currentValue;
}

// See https://github.com/mui/material-ui/issues/41190#issuecomment-2040873379 for why
const safeReact = { ...React };
const maybeReactUseDeferredValue: undefined | typeof React.useDeferredValue =
safeReact.useDeferredValue;

const useDeferredValue =
typeof maybeReactUseDeferredValue === 'undefined'
? useDeferredValue17
: maybeReactUseDeferredValue;

export default useDeferredValue;
2 changes: 2 additions & 0 deletions packages/mui-utils/src/useReducerWithTransitions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use client';
export { default } from './useReducerWithTransitions';
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as React from 'react';
import {
runWithBatch,
blockingBatchCallback,
enqueueStateUpdate,
} from '../CompatTransitionManager';

/**
* Like `useReducer`, but for use with the compatibility version of `useTransition`.
* TODO: Improve typing.
*/
export default function useReducerWithTransitions<T>(
reducer: any,
initializerArg: any,
initializer?: any,
) {
const [state, dispatch] = React.useReducer(reducer, initializerArg, initializer);
const enqueueDispatch = React.useCallback(
(value: any) => {
runWithBatch(() => {
enqueueStateUpdate(() => (dispatch as any)(value));
}, blockingBatchCallback);
},
[dispatch],
);

return [state, enqueueDispatch];
}
2 changes: 2 additions & 0 deletions packages/mui-utils/src/useStateWithTransitions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use client';
export { default } from './useStateWithTransitions';
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as React from 'react';
import {
runWithBatch,
blockingBatchCallback,
enqueueStateUpdate,
} from '../CompatTransitionManager';

type StateUpdateFn<T> = (prevState: T) => T;
type SetStateInput<T> = T | StateUpdateFn<T>;
type SetStateFn<T> = (value: SetStateInput<T>) => void;

/**
* Like `useState`, but for use with the compatibility version of `useTransition`.
*/
export default function useStateWithTransitions<T>(
initialValue: T | (() => T),
): [T, SetStateFn<T>] {
const [state, setState] = React.useState(initialValue);
const enqueueSetState = React.useCallback(
(value: SetStateInput<T>) => {
runWithBatch(() => {
enqueueStateUpdate(() => setState(value));
}, blockingBatchCallback);
},
[setState],
);

return [state, enqueueSetState];
}
2 changes: 2 additions & 0 deletions packages/mui-utils/src/useTransition/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use client';
export { default } from './useTransition';
Loading