Skip to content

perf(compact-select): render options lazily #103811

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.

Sign up for GitHub

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

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
+184 −200
Open

perf(compact-select): render options lazily #103811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
ecdc9b1
perf: do not render <List> inside CompactSelect when the overlay is c…
TkDodo Nov 19, 2025
f6f4975
Merge branch 'master' into tkdodo/perf/compact-select
TkDodo Nov 19, 2025
562b528
ref: defaultValue is no more
TkDodo Nov 19, 2025
984999b
ref: make items and value optional for `control`
TkDodo Nov 19, 2025
65eca0f
ref: move conditional rendering further down
TkDodo Nov 19, 2025
215698d
Merge branch 'master' into tkdodo/perf/compact-select
TkDodo Nov 19, 2025
a3b60f7
ref: compositeSelect gets a required trigger
TkDodo Nov 20, 2025
8055756
ref: fix tests
TkDodo Nov 20, 2025
13f258e
Merge branch 'master' into tkdodo/ref/composite-select-required-trigger
TkDodo Nov 20, 2025
0860554
merge branch ref/composite-select-required-trigger
TkDodo Nov 20, 2025
5c5c864
fix: move conditional rendering to the highest possible point
TkDodo Nov 20, 2025
31c671e
fix: we need proper state management to make selection work now
TkDodo Nov 20, 2025
91179cc
merge branch master
TkDodo Nov 21, 2025
988d997
ref: remove compositeIndex
TkDodo Nov 21, 2025
b18d6e0
fix: inline onClear and remove call to non-existing setSelectedOptions
TkDodo Nov 21, 2025
93f5e34
fix: hasSelection logic
TkDodo Nov 21, 2025
5518059
test: adapt tests
TkDodo Nov 21, 2025
c32c0b0
Merge branch 'master' into tkdodo/perf/compact-select
TkDodo Nov 21, 2025
dc6b4d7
test: more test fixes
TkDodo Nov 21, 2025
f83ffd5
Merge branch 'master' into tkdodo/perf/compact-select
TkDodo Nov 21, 2025
4b1bf62
options?.flat
TkDodo Nov 21, 2025
5317c59
Merge branch 'master' into tkdodo/perf/compact-select
TkDodo Nov 21, 2025
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
16 changes: 6 additions & 10 deletions static/app/components/core/compactSelect/compactSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,10 @@ interface BaseSelectProps<Value extends SelectKey>
}

export type SingleSelectProps<Value extends SelectKey> = BaseSelectProps<Value> &
DistributiveOmit<
SingleListProps<Value>,
'children' | 'items' | 'grid' | 'compositeIndex' | 'label'
>;
DistributiveOmit<SingleListProps<Value>, 'children' | 'items' | 'grid' | 'label'>;

export type MultipleSelectProps<Value extends SelectKey> = BaseSelectProps<Value> &
DistributiveOmit<
MultipleListProps<Value>,
'children' | 'items' | 'grid' | 'compositeIndex' | 'label'
>;
DistributiveOmit<MultipleListProps<Value>, 'children' | 'items' | 'grid' | 'label'>;

export type SelectProps<Value extends SelectKey> =
| SingleSelectProps<Value>
Expand Down Expand Up @@ -126,6 +120,8 @@ export function CompactSelect<Value extends SelectKey>({
disabled={controlDisabled}
grid={grid}
size={size}
items={itemsWithKey}
value={value}
clearable={clearable}
onClear={({overlayState}) => {
if (clearable) {
Expand Down Expand Up @@ -154,11 +150,11 @@ export function CompactSelect<Value extends SelectKey>({
if ('options' in item) {
return (
<Section key={item.key} title={item.label}>
{item.options.map(opt => (
{item.options?.map(opt => (
Copy link
Member

@JonasBa JonasBa Nov 21, 2025

Choose a reason for hiding this comment

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

I would expect the check above to narrow it down to Array, is this an expected change?

Copy link
Member

@natemoo-re natemoo-re Nov 21, 2025

Choose a reason for hiding this comment

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

I think that's expected as items support an explicit undefined now?

<Item {...opt} key={opt.key}>
{opt.label}
</Item>
))}
)) ?? null}
</Section>
);
}
Expand Down
21 changes: 5 additions & 16 deletions static/app/components/core/compactSelect/composite.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ interface BaseCompositeSelectRegion<Value extends SelectKey> {
*/
type SingleCompositeSelectRegion<Value extends SelectKey> =
BaseCompositeSelectRegion<Value> &
DistributiveOmit<
SingleListProps<Value>,
'children' | 'items' | 'grid' | 'compositeIndex' | 'size'
>;
DistributiveOmit<SingleListProps<Value>, 'children' | 'items' | 'grid' | 'size'>;

/**
* A multiple-selection (multiple options can be selected at the same time) "region"
Expand All @@ -43,10 +40,7 @@ type SingleCompositeSelectRegion<Value extends SelectKey> =
*/
type MultipleCompositeSelectRegion<Value extends SelectKey> =
BaseCompositeSelectRegion<Value> &
DistributiveOmit<
MultipleListProps<Value>,
'children' | 'items' | 'grid' | 'compositeIndex' | 'size'
>;
DistributiveOmit<MultipleListProps<Value>, 'children' | 'items' | 'grid' | 'size'>;

/**
* A "region" inside a composite select. Each "region" is a separated, self-contained
Expand All @@ -68,7 +62,7 @@ type CompositeSelectChild =
| undefined;

export interface CompositeSelectProps
extends Omit<ControlProps, 'triggerProps' | 'trigger'> {
extends Omit<ControlProps, 'clearable' | 'triggerProps' | 'trigger'> {
/**
* The "regions" inside this composite selector. Each region functions as a separated,
* self-contained selectable list (each renders as a `ul` with its own list state)
Expand All @@ -94,14 +88,12 @@ function CompositeSelect({
<Control {...controlProps} grid={grid} size={size} disabled={disabled}>
<FocusScope>
<RegionsWrap>
{Children.map(children, (child, index) => {
{Children.map(children, child => {
if (!child) {
return null;
}

return (
<Region {...child.props} grid={grid} size={size} compositeIndex={index} />
);
return <Region {...child.props} grid={grid} size={size} />;
})}

{/* Only displayed when all lists (regions) are empty */}
Expand Down Expand Up @@ -129,7 +121,6 @@ CompositeSelect.Region = function <Value extends SelectKey>(
export {CompositeSelect};

type RegionProps<Value extends SelectKey> = CompositeSelectRegion<Value> & {
compositeIndex: SingleListProps<Value>['compositeIndex'];
grid: SingleListProps<Value>['grid'];
size: SingleListProps<Value>['size'];
};
Expand All @@ -138,7 +129,6 @@ function Region<Value extends SelectKey>({
options,
isOptionDisabled,
size,
compositeIndex,
label,
...props
}: RegionProps<Value>) {
Expand All @@ -150,7 +140,6 @@ function Region<Value extends SelectKey>({
items={itemsWithKey}
isOptionDisabled={isOptionDisabled}
shouldFocusWrap={false}
compositeIndex={compositeIndex}
size={size}
label={label}
>
Expand Down
Loading
Loading