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

Use generics for Select arrays #7036

Merged
merged 4 commits into from
Oct 31, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ const INTENTS = [Intent.NONE, Intent.PRIMARY, Intent.SUCCESS, Intent.DANGER, Int

export interface MultiSelectExampleState {
allowCreate: boolean;
createdItems: Film[];
createdItems: readonly Film[];
Copy link
Contributor Author

@ggdouglas ggdouglas Oct 30, 2024

Choose a reason for hiding this comment

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

The modification of the examples here in this PR isn't strictly necessary, though it helps to serve as a practical example of how to migrate existing Select/MultiSelect components to use readonly arrays. It's also worth noting that the examples here still compile fine if they are not switched to use readonly arrays, demonstrating the backwards compatibility of these type changes.

All the docs/test changes are contained within: 19cdbee

disabled: boolean;
fill: boolean;
films: Film[];
films: readonly Film[];
hasInitialContent: boolean;
intent: boolean;
items: Film[];
items: readonly Film[];
matchTargetWidth: boolean;
openOnKeyDown: boolean;
popoverMinimal: boolean;
Expand Down Expand Up @@ -124,7 +124,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, MultiS

return (
<Example options={this.renderOptions()} {...this.props}>
<MultiSelect<Film>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The omission of the specified Film generic parameter to the component is necessary here, otherwise we'll get a type error:

'readonly Film[]' is 'readonly' and cannot be assigned to the mutable type 'Film[]'

Another way to solve this is to explicitly define the second type parameter, e.g.

<MultiSelect<Film, readonly Film[]>

<MultiSelect
{...flags}
createNewItemFromQuery={allowCreate ? createFilms : undefined}
createNewItemRenderer={allowCreate ? renderCreateFilmsMenuItem : null}
Expand Down Expand Up @@ -251,7 +251,9 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, MultiS
);
}

private renderCustomTarget = (selectedItems: Film[]) => <MultiSelectCustomTarget count={selectedItems.length} />;
private renderCustomTarget = (selectedItems: readonly Film[]) => (
<MultiSelectCustomTarget count={selectedItems.length} />
);

private renderTag = (film: Film) => film.title;

Expand Down Expand Up @@ -287,11 +289,11 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, MultiS
this.selectFilms([film]);
}

private selectFilms(filmsToSelect: Film[]) {
private selectFilms(filmsToSelect: readonly Film[]) {
this.setState(({ createdItems, films, items }) => {
let nextCreatedItems = createdItems.slice();
let nextFilms = films.slice();
let nextItems = items.slice();
let nextCreatedItems = createdItems;
let nextFilms = films;
let nextItems = items;

filmsToSelect.forEach(film => {
const results = maybeAddCreatedFilmToArrays(nextItems, nextCreatedItems, film);
Expand Down Expand Up @@ -336,7 +338,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, MultiS
}
};

private handleFilmsPaste = (films: Film[]) => {
private handleFilmsPaste = (films: readonly Film[]) => {
// On paste, don't bother with deselecting already selected values, just
// add the new ones.
this.selectFilms(films);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class OmnibarExample extends React.PureComponent<ExampleProps, OmnibarExa
<KeyComboTag combo="shift + o" />
</span>

<Omnibar<Film>
<Omnibar
{...this.state}
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
Expand Down
10 changes: 5 additions & 5 deletions packages/docs-app/src/examples/select-examples/selectExample.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { type Film, FilmSelect, filterFilm, TOP_100_FILMS } from "@blueprintjs/s
export interface SelectExampleState {
allowCreate: boolean;
createFirst: boolean;
createdItems: Film[];
createdItems: readonly Film[];
disableItems: boolean;
disabled: boolean;
fill: boolean;
Expand Down Expand Up @@ -169,7 +169,7 @@ export class SelectExample extends React.PureComponent<ExampleProps, SelectExamp
return /[0-9]/.test(firstLetter) ? "0-9" : firstLetter;
}

private getGroupedItems = (filteredItems: Film[]) => {
private getGroupedItems = (filteredItems: readonly Film[]) => {
return filteredItems.reduce<Array<{ group: string; index: number; items: Film[]; key: number }>>(
(acc, item, index) => {
const group = this.getGroup(item);
Expand All @@ -193,7 +193,7 @@ export class SelectExample extends React.PureComponent<ExampleProps, SelectExamp
) : undefined;
};

private groupedItemListPredicate = (query: string, items: Film[]) => {
private groupedItemListPredicate = (query: string, items: readonly Film[]) => {
return items
.filter((item, index) => filterFilm(query, item, index))
.sort((a, b) => this.getGroup(a).localeCompare(this.getGroup(b)));
Expand All @@ -208,7 +208,7 @@ export class SelectExample extends React.PureComponent<ExampleProps, SelectExamp

private isItemDisabled = (film: Film) => this.state.disableItems && film.year < 2000;

private renderGroupedItemList = (listProps: ItemListRendererProps<Film>) => {
private renderGroupedItemList = (listProps: ItemListRendererProps<Film, readonly Film[]>) => {
const initialContent = this.getInitialContent();
const noResults = <MenuItem disabled={true} text="No results." roleStructure="listoption" />;

Expand All @@ -231,7 +231,7 @@ export class SelectExample extends React.PureComponent<ExampleProps, SelectExamp
};

private renderGroupedMenuContent = (
listProps: ItemListRendererProps<Film>,
listProps: ItemListRendererProps<Film, readonly Film[]>,
noResults?: React.ReactNode,
initialContent?: React.ReactNode | null,
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ import {
export interface SuggestExampleState {
allowCreate: boolean;
closeOnSelect: boolean;
createdItems: Film[];
createdItems: readonly Film[];
disabled: boolean;
fill: boolean;
items: Film[];
items: readonly Film[];
matchTargetWidth: boolean;
minimal: boolean;
openOnKeyDown: boolean;
Expand Down Expand Up @@ -92,7 +92,7 @@ export class SuggestExample extends React.PureComponent<ExampleProps, SuggestExa

return (
<Example options={this.renderOptions()} {...this.props}>
<Suggest<Film>
<Suggest
{...flags}
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
Expand Down
8 changes: 4 additions & 4 deletions packages/select/src/__examples__/filmSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
} from "./films";

type FilmSelectProps = Omit<
SelectProps<Film>,
SelectProps<Film, readonly Film[]>,
| "createNewItemFromQuery"
| "createNewItemRenderer"
| "itemPredicate"
Expand All @@ -49,8 +49,8 @@ type FilmSelectProps = Omit<
};

export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSelectProps) {
const [items, setItems] = React.useState([...TOP_100_FILMS]);
const [createdItems, setCreatedItems] = React.useState<Film[]>([]);
const [items, setItems] = React.useState<readonly Film[]>([...TOP_100_FILMS]);
const [createdItems, setCreatedItems] = React.useState<readonly Film[]>([]);
const [selectedFilm, setSelectedFilm] = React.useState<Film | undefined>(undefined);
const handleItemSelect = React.useCallback(
(newFilm: Film) => {
Expand Down Expand Up @@ -82,7 +82,7 @@ export function FilmSelect({ allowCreate = false, fill, ...restProps }: FilmSele
);

return (
<Select<Film>
<Select
createNewItemFromQuery={allowCreate ? createFilm : undefined}
createNewItemRenderer={allowCreate ? renderCreateFilmMenuItem : undefined}
fill={fill}
Expand Down
22 changes: 11 additions & 11 deletions packages/select/src/__examples__/films.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface Film {
}

/** Top 100 films as rated by IMDb users. http://www.imdb.com/chart/top */
export const TOP_100_FILMS: Film[] = [
export const TOP_100_FILMS: readonly Film[] = [
{ title: "The Shawshank Redemption", year: 1994 },
{ title: "The Godfather", year: 1972 },
{ title: "The Godfather: Part II", year: 1974 },
Expand Down Expand Up @@ -270,7 +270,7 @@ export function createFilm(title: string): Film {
};
}

export function createFilms(query: string): Film[] {
export function createFilms(query: string): readonly Film[] {
const titles = query.split(", ");
return titles.map((title, index) => ({
rank: 100 + Math.floor(Math.random() * 100 + index),
Expand All @@ -288,23 +288,23 @@ export function doesFilmEqualQuery(film: Film, query: string) {
return film.title.toLowerCase() === query.toLowerCase();
}

export function arrayContainsFilm(films: Film[], filmToFind: Film): boolean {
export function arrayContainsFilm(films: readonly Film[], filmToFind: Film): boolean {
return films.some((film: Film) => film.title === filmToFind.title);
}

export function addFilmToArray(films: Film[], filmToAdd: Film) {
export function addFilmToArray(films: readonly Film[], filmToAdd: Film): readonly Film[] {
return [...films, filmToAdd];
}

export function deleteFilmFromArray(films: Film[], filmToDelete: Film) {
export function deleteFilmFromArray(films: readonly Film[], filmToDelete: Film): readonly Film[] {
return films.filter(film => film !== filmToDelete);
}

export function maybeAddCreatedFilmToArrays(
items: Film[],
createdItems: Film[],
items: readonly Film[],
createdItems: readonly Film[],
film: Film,
): { createdItems: Film[]; items: Film[] } {
): { createdItems: readonly Film[]; items: readonly Film[] } {
const isNewlyCreatedItem = !arrayContainsFilm(items, film);
return {
createdItems: isNewlyCreatedItem ? addFilmToArray(createdItems, film) : createdItems,
Expand All @@ -314,10 +314,10 @@ export function maybeAddCreatedFilmToArrays(
}

export function maybeDeleteCreatedFilmFromArrays(
items: Film[],
createdItems: Film[],
items: readonly Film[],
createdItems: readonly Film[],
film: Film | undefined,
): { createdItems: Film[]; items: Film[] } {
): { createdItems: readonly Film[]; items: readonly Film[] } {
if (film === undefined) {
return {
createdItems,
Expand Down
14 changes: 8 additions & 6 deletions packages/select/src/common/itemListRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type { CreateNewItem } from "./listItemsUtils";
* An object describing how to render the list of items.
* An `itemListRenderer` receives this object as its sole argument.
*/
export interface ItemListRendererProps<T> {
export interface ItemListRendererProps<T, A extends readonly T[] = T[]> {
/**
* The currently focused item (for keyboard interactions), or `null` to
* indicate that no item is active.
Expand All @@ -35,13 +35,13 @@ export interface ItemListRendererProps<T> {
* map each item in this array through `renderItem`, with support for
* optional `noResults` and `initialContent` states.
*/
filteredItems: T[];
filteredItems: A;

/**
* Array of all items in the list.
* See `filteredItems` for a filtered array based on `query` and predicate props.
*/
items: T[];
items: A;

/**
* The current query string.
Expand Down Expand Up @@ -75,15 +75,17 @@ export interface ItemListRendererProps<T> {
}

/** Type alias for a function that renders the list of items. */
export type ItemListRenderer<T> = (itemListProps: ItemListRendererProps<T>) => React.JSX.Element | null;
export type ItemListRenderer<T, A extends readonly T[] = T[]> = (
itemListProps: ItemListRendererProps<T, A>,
) => React.JSX.Element | null;

/**
* `ItemListRenderer` helper method for rendering each item in `filteredItems`,
* with optional support for `noResults` (when filtered items is empty)
* and `initialContent` (when query is empty).
*/
export function renderFilteredItems(
props: ItemListRendererProps<any>,
export function renderFilteredItems<T, A extends readonly T[] = T[]>(
props: ItemListRendererProps<T, A>,
noResults?: React.ReactNode,
initialContent?: React.ReactNode | null,
): React.ReactNode {
Expand Down
12 changes: 6 additions & 6 deletions packages/select/src/common/listItemsProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export type ItemsEqualComparator<T> = (itemA: T, itemB: T) => boolean;
export type ItemsEqualProp<T> = ItemsEqualComparator<T> | keyof T;

/** Reusable generic props for a component that operates on a filterable, selectable list of `items`. */
export interface ListItemsProps<T> extends Props {
export interface ListItemsProps<T, A extends readonly T[] = T[]> extends Props {
/**
* The currently focused item for keyboard interactions, or `null` to
* indicate that no item is active. If omitted or `undefined`, this prop will be
Expand All @@ -44,7 +44,7 @@ export interface ListItemsProps<T> extends Props {
activeItem?: T | CreateNewItem | null;

/** Array of items in the list. */
items: T[];
items: A;

/**
* Specifies how to test if two items are equal. By default, simple strict
Expand Down Expand Up @@ -75,7 +75,7 @@ export interface ListItemsProps<T> extends Props {
*
* If `itemPredicate` is also defined, this prop takes priority and the other will be ignored.
*/
itemListPredicate?: ItemListPredicate<T>;
itemListPredicate?: ItemListPredicate<T, A>;

/**
* Customize querying of individual items.
Expand Down Expand Up @@ -110,7 +110,7 @@ export interface ListItemsProps<T> extends Props {
* and wraps them all in a `Menu` element. If the query is empty then `initialContent` is returned,
* and if there are no items that match the predicate then `noResults` is returned.
*/
itemListRenderer?: ItemListRenderer<T>;
itemListRenderer?: ItemListRenderer<T, A>;

/**
* React content to render when query is empty.
Expand Down Expand Up @@ -157,7 +157,7 @@ export interface ListItemsProps<T> extends Props {
/**
* Callback invoked when multiple items are selected at once via pasting.
*/
onItemsPaste?: (items: T[]) => void;
onItemsPaste?: (items: A) => void;

/**
* Callback invoked when the query string changes.
Expand All @@ -170,7 +170,7 @@ export interface ListItemsProps<T> extends Props {
* created, either by pressing the `Enter` key or by clicking on the "Create
* Item" option. It transforms a query string into one or many items type.
*/
createNewItemFromQuery?: (query: string) => T | T[];
createNewItemFromQuery?: (query: string) => T | A;

/**
* Custom renderer to transform the current query string into a selectable
Expand Down
2 changes: 1 addition & 1 deletion packages/select/src/common/predicate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* A custom predicate for returning an entirely new `items` array based on the provided query.
* See usage sites in `ListItemsProps`.
*/
export type ItemListPredicate<T> = (query: string, items: T[]) => T[];
export type ItemListPredicate<T, A extends readonly T[] = T[]> = (query: string, items: A) => A;

/**
* A custom predicate for filtering items based on the provided query.
Expand Down
Loading