Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 27 additions & 1 deletion eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ export default defineConfig(
languageOptions: {
parserOptions: {
tsconfigRootDir: dirname,
project: ['./tsconfig.json'],
projectService: true,
},
},
},
Expand Down Expand Up @@ -335,6 +335,32 @@ export default defineConfig(
},
},

// Catch leaked subscriptions: call statements whose returned cleanup /
// unsubscribe function is discarded. Type-aware, so it needs TypeScript type
// information (same `projectService` setup as `mui-x/no-direct-state-access` above).
{
files: [`packages/*/src/**/*${EXTENSION_TS}`],
ignores: [
'**/*.d.ts',
`**/*.spec${EXTENSION_TS}`,
`**/*.test${EXTENSION_TS}`,
// Codemods are jscodeshift AST transforms with no runtime subscriptions;
// the only hits are chai assertions in a test-style file.
'packages/x-codemod/**',
// Vendored copy of Base UI internals — keep in sync with upstream, don't edit.
'packages/x-scheduler-internals/src/base-ui-copy/**',
],
languageOptions: {
parserOptions: {
tsconfigRootDir: dirname,
projectService: true,
},
},
rules: {
'mui/no-floating-cleanup': 'error',
},
},

// Common config from core start
{
files: [`docs/**/*${EXTENSION_TS}`],
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"@mui/internal-babel-plugin-display-name": "1.0.4-canary.20",
"@mui/internal-babel-plugin-minify-errors": "2.0.8-canary.27",
"@mui/internal-bundle-size-checker": "1.0.9-canary.81",
"@mui/internal-code-infra": "0.0.4-canary.64",
"@mui/internal-code-infra": "0.0.4-canary.66",
"@mui/internal-api-docs-builder": "1.0.2-canary.2",
"@mui/internal-markdown": "3.0.9-canary.2",
"@mui/internal-netlify-cache": "0.0.3-canary.5",
Expand Down
4 changes: 3 additions & 1 deletion packages/x-charts/src/internals/scales/scaleBand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ function createScaleBand(seed?: ScaleBandState): ScaleBand<any> {
scale.copy = () =>
createScaleBand({ index, domain, r0, r1, isRound, paddingInner, paddingOuter, align });

rescale();
// `rescale` returns the scale for the fluent setters; here it only seeds the
// initial layout, so the (callable) return is intentionally discarded.
void rescale();

return scale as any;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/x-chat/src/ChatBox/ChatBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ const ChatBox = React.forwardRef(function ChatBox<Cursor = string>(
(node: HTMLDivElement | null) => {
setRootElement(node);
if (typeof ref === 'function') {
ref(node);
// Legacy callback-ref forwarding; a React 19 cleanup return is not propagated here.
void ref(node);
} else if (ref) {
(ref as React.MutableRefObject<HTMLDivElement | null>).current = node;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/x-chat/src/ChatBox/ChatBoxContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ const ChatBoxConversationOverlay = styled('div', {
// conversations pane. Marking it keeps ChatLayout's pane resolution unambiguous —
// once any sibling (the thread view) is marked, every direct child must be marked
// or ChatLayout warns about a mixed/undeterminable set.
markChatLayoutPane(ChatBoxConversationOverlay, 'conversations');
// `markChatLayoutPane` mutates the component in place and returns it; the return is discarded here.
void markChatLayoutPane(ChatBoxConversationOverlay, 'conversations');

const ChatBoxConversationOverlayBackdrop = styled('div', {
name: 'MuiChatBox',
Expand Down Expand Up @@ -632,7 +633,7 @@ function createMarkedConversationListComponent(
return <Component ref={ref} {...props} />;
},
);
markChatLayoutPane(MarkedConversationList, 'conversations');
void markChatLayoutPane(MarkedConversationList, 'conversations');
return MarkedConversationList as typeof ChatConversationList;
}

Expand Down
4 changes: 3 additions & 1 deletion packages/x-chat/src/ChatConversation/ChatConversation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ ChatConversation.propTypes /* remove-proptypes */ = {
// Mirror the headless `ConversationRoot` pane marker on the Material wrapper so
// `ChatLayout` assigns it to the thread pane (the symbol lives on the headless
// primitive, not this wrapper).
markChatLayoutPane(ChatConversation, 'thread');
// `markChatLayoutPane` mutates the component in place and returns it for the
// `const X = markChatLayoutPane(...)` form; here the return is intentionally discarded.
void markChatLayoutPane(ChatConversation, 'thread');

export { ChatConversation };
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ ChatConversationList.propTypes /* remove-proptypes */ = {
// primitive, not this wrapper). Without it, a split layout with the list visible and
// no active conversation — a single unmarked child — falls back to the thread pane,
// skipping the `conversationsPane` slot/styles.
markChatLayoutPane(ChatConversationList, 'conversations');
// `markChatLayoutPane` mutates the component in place and returns it; the return is discarded here.
void markChatLayoutPane(ChatConversationList, 'conversations');

export { ChatConversationList };
3 changes: 2 additions & 1 deletion packages/x-chat/src/internals/mergeSlotProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ function setRef(ref: React.Ref<unknown>, value: unknown) {
return;
}
if (typeof ref === 'function') {
ref(value);
// Legacy callback-ref forwarding; a React 19 cleanup return is not propagated here.
void ref(value);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export const useKeepGroupedColumnsHidden = (
);

React.useEffect(() => {
props.apiRef.current?.subscribeEvent('rowGroupingModelChange', (newModel) => {
return props.apiRef.current?.subscribeEvent('rowGroupingModelChange', (newModel) => {
const columnVisibilityModel = updateColumnVisibilityModel(
gridColumnVisibilityModelSelector(props.apiRef),
newModel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ export const useGridRegisterStrategyProcessor = <
processor: GridStrategyProcessor<G>,
) => {
const registerPreProcessor = React.useCallback(() => {
apiRef.current.registerStrategyProcessor(strategyName, group, processor);
// FIXME: the unregister fn is discarded — unlike `useGridRegisterPipeProcessor`,
// which stores and calls it. Strategy processors currently must outlive the
// component: tearing one down on unmount breaks strategy resolution (e.g.
// "No processor found" on tree-data filtering/sorting). `void` until resolved.
void apiRef.current.registerStrategyProcessor(strategyName, group, processor);
Comment thread
romgrk marked this conversation as resolved.
}, [apiRef, processor, group, strategyName]);

useFirstRender(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,11 @@ You need to upgrade to DataGridPro or DataGridPremium component to unlock multip
}, [apiRef, canHaveMultipleSelection, checkboxSelection, isStateControlled, props.rowSelection]);

React.useEffect(() => {
runIf(props.rowSelection, removeOutdatedSelection);
// FIXME: no-op since #20668. `runIf` is curried, so this builds a wrapper and
// discards it (previously `runIfRowSelectionIsEnabled(removeOutdatedSelection)`,
// which invoked it). Restoring the call changes selection behavior, so it needs
// a decision; `void` preserves current behavior. See #20668.
void runIf(props.rowSelection, removeOutdatedSelection);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no-floating-cleanup flagged this. runIf is curried — runIf(props.rowSelection, removeOutdatedSelection) builds a handler and returns it, so as a bare statement the effect is a no-op. It regressed in #20668, which replaced runIfRowSelectionIsEnabled(removeOutdatedSelection) (that one invoked the fn).

Restoring the call (if (props.rowSelection) removeOutdatedSelection()) makes rowSelection.DataGridPro › "should auto select parents when controlling row selection model" expect an extra onRowSelectionModelChange (4 vs 3) — so whether the effect should run is a behavior decision. Left as void to preserve current behavior; needs a maintainer call on whether to restore it (and update the test) or drop the dead effect.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Resolved in 1d7dba4: deleted the dead effect. Since runIf is curried it has been a no-op since #20668, so removing it is behavior-identical and drops the dead code. removeOutdatedSelection stays wired to the filteredRowsSet event. (If prop-change re-pruning is actually wanted, that is a separate, deliberate fix that also needs to suppress the redundant onRowSelectionModelChange — happy to open an issue.)

}, [props.rowSelection, removeOutdatedSelection]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this cause trouble?
Not sure if it is redundant or if we are just missing the test for an edge case


React.useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,12 @@ export class EventTimelinePremiumStore<
if (process.env.NODE_ENV !== 'production') {
// Assert the initial state validity; `subscribe` only fires on subsequent state changes.
this.assertPresetValidity(this.state.preset);
this.subscribe((state) => {
this.assertPresetValidity(state.preset);
return null;
});
this.disposables.defer(
this.subscribe((state) => {
this.assertPresetValidity(state.preset);
return null;
}),
);
}

this.lazyLoading = this.disposables.use(new EventTimelinePremiumLazyLoadingPlugin(this));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,12 @@ export class ExtendableEventCalendarStore<
if (process.env.NODE_ENV !== 'production') {
// Assert the initial state validity; `subscribe` only fires on subsequent state changes.
this.assertViewValidity(this.state.view);
this.subscribe((state) => {
this.assertViewValidity(state.view);
return null;
});
this.disposables.defer(
this.subscribe((state) => {
this.assertViewValidity(state.view);
return null;
}),
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default function MoreEventsPopoverContent(props: MoreEventsPopoverProps)
const { subscribeCloseHandler } = useEventDialogContext();

React.useEffect(() => {
subscribeCloseHandler(() => {
return subscribeCloseHandler(() => {
onClose();
});
}, [subscribeCloseHandler, onClose]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ export class RichTreeViewProStore<

public itemsReordering = new TreeViewItemsReorderingPlugin(this);

public disposeEffect = () => {
public mountEffect = () => {
this.lazyLoading.initEffect();
return this.timeoutManager.clearAll;
};

public constructor(parameters: RichTreeViewProStoreParameters<R, Multiple>) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { Store } from '@mui/x-internals/store';
import { warnOnce } from '@mui/x-internals/warning';
import { EventManager } from '@mui/x-internals/EventManager';
import {
DisposableStack,
disposeSymbol,
unwrapSuppressedErrors,
} from '@mui/x-internals/disposable';
import {
TreeViewModelUpdater,
MinimalTreeViewParameters,
Expand Down Expand Up @@ -40,13 +45,23 @@ export class MinimalTreeViewStore<

private mapper: TreeViewParametersToStateMapper<R, Multiple, State, Parameters>;

private eventManager = new EventManager();
// Owns the store's teardown. Declared first so the resources below register
// against it during field initialization; disposed by `useDisposable` on
// unmount (see `[disposeSymbol]`). `public` so plugins can register their own
// subscriptions against it (hidden from the context store type).
public readonly disposables = new DisposableStack();

private eventManager = this.disposables.adopt(new EventManager(), (manager) =>
manager.removeAllListeners(),
);

public instanceName: string;

public parameters: Parameters;

public timeoutManager = new TimeoutManager();
public timeoutManager = this.disposables.adopt(new TimeoutManager(), (manager) =>
manager.clearAll(),
);

public itemPluginManager = new TreeViewItemPluginManager<this>();

Expand Down Expand Up @@ -168,11 +183,32 @@ export class MinimalTreeViewStore<
}

/**
* Returns a cleanup function that need to be called when the store is destroyed.
* Runs mount-time side effects that must not happen during render (the store
* is created during render by `useDisposable`). No-op by default; overridden
* by stores that kick off work on mount (e.g. lazy-loading fetches). Safe to
* call more than once (StrictMode replays mount effects).
*/
public disposeEffect = () => {
return this.timeoutManager.clearAll;
};
public mountEffect = () => {};

/**
* Disposes the store synchronously when the component unmounts. `useDisposable`
* handles React StrictMode's simulated unmount, so this runs once on real unmount.
*/
[disposeSymbol](): void {
if (this.disposables.disposed) {
return;
}
try {
this.disposables.dispose();
} catch (error) {
if (process.env.NODE_ENV !== 'production') {
console.error(
'MUI X Tree View: error while disposing the store.',
...unwrapSuppressedErrors(error),
);
}
}
}

/**
* Whether updates based on `props.items` change should be ignored.
Expand All @@ -190,13 +226,15 @@ export class MinimalTreeViewStore<
) => {
let previousValue = selector(this.state);

this.subscribe((state) => {
const nextValue = selector(state);
if (nextValue !== previousValue) {
effect(previousValue, nextValue);
previousValue = nextValue;
}
});
this.disposables.defer(
this.subscribe((state) => {
const nextValue = selector(state);
if (nextValue !== previousValue) {
effect(previousValue, nextValue);
previousValue = nextValue;
}
}),
);
};

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from 'react';
import { disposeSymbol } from '@mui/x-internals/disposable';
import {
TreeItemWrapper,
TreeViewItemPluginResponse,
Expand All @@ -19,7 +20,9 @@ export type TreeViewStoreInContext<TStore extends TreeViewAnyStore> = Omit<
| 'update'
| 'set'
| 'updateStateFromParameters'
| 'disposeEffect'
| 'disposables'
| typeof disposeSymbol
| 'mountEffect'
| 'registerStoreEffect'
| 'itemPluginManager'
| 'parameters'
Expand Down
8 changes: 5 additions & 3 deletions packages/x-tree-view/src/internals/hooks/useTreeViewStore.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use client';
import { useRefWithInit } from '@base-ui/utils/useRefWithInit';
import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect';
import { useOnMount } from '@base-ui/utils/useOnMount';
import { useDisposable } from '@mui/x-internals/useDisposable';
import { useRtl } from '@mui/system/RtlProvider';
import { TreeViewAnyStore } from '../models';

Expand All @@ -22,14 +22,16 @@ export function useTreeViewStore<TStore extends TreeViewAnyStore>(
parameters: UseTreeViewStoreParameters<TStore>,
): TStore {
const isRtl = useRtl();
const store = useRefWithInit(() => new StoreClass({ ...parameters, isRtl })).current;
const store = useDisposable(() => new StoreClass({ ...parameters, isRtl }));

useIsoLayoutEffect(
() => store.updateStateFromParameters({ ...parameters, isRtl }),
[store, isRtl, parameters],
);

useOnMount(store.disposeEffect);
// Mount-time side effects (e.g. kicking off lazy-loading fetches). The store is
// created during render by `useDisposable`, so these can't run in the factory.
useOnMount(store.mountEffect);

return store;
}
Loading
Loading