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

[Table] Batch Child Rendering #1201

Merged
merged 18 commits into from
Jun 12, 2017
Merged
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
Prev Previous commit
Next Next commit
PR Comments
  • Loading branch information
themadcreator committed Jun 8, 2017
commit 1752ff1d152ed82689fdbbb61fd7210534ddc240
33 changes: 14 additions & 19 deletions packages/table/src/common/batcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@

import { requestIdleCallback } from "./requestIdleCallback";

export interface IIndex<T> {
[key: string]: T;
}

export type SimpleStringifyable = string | number | null;
export type SimpleStringifyable = string | number | null | undefined;

export type Callback = () => void;

Expand All @@ -20,17 +16,16 @@ export type Callback = () => void;
*
* For example, if your react component has many children, updating them all at
Copy link
Contributor

Choose a reason for hiding this comment

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

ubernit: React

* once may cause jank when reconciling the DOM. This class helps you update
* only a few per frame.
* only a few children per frame.
*
* A typical usage would be:
*
* ```typescript
*
* ```tsx
* public renderChildren = (allChildrenKeys: string[]) => {
*
* batcher.startNewBatch();
*
* allChildrenKeys.forEach((prop1, index) => {
* allChildrenKeys.forEach((prop1: string, index: number) => {
* batcher.addArgsToBatch(prop1, "prop2", index);
Copy link
Contributor

Choose a reason for hiding this comment

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

prop1, "prop2" 1, 2 intentional?

* });
*
Expand All @@ -52,8 +47,8 @@ export class Batcher<T> {
public static DEFAULT_ADD_LIMIT = 20;
public static DEFAULT_REMOVE_LIMIT = 20;

private currentObjects: IIndex<T> = {};
private batchArgs: IIndex<any[]> = {};
private currentObjects: Record<string, T> = {};
private batchArgs: Record<string, any[]> = {};
private done = true;
private callback: Callback;

Expand Down Expand Up @@ -86,13 +81,13 @@ export class Batcher<T> {
* Compares the set of "batch" arguments to the "current" set. Creates any
* new objects using the callback as a factory. Removes old objects.
*
* Arguments are in the "current" set but were not part of the last "batch"
* set are considered candidates for removal. Similarly, Arguments that are
* part of the "batch" set but not the "current" set are candidates for
* addition.
* Arguments that are in the "current" set but were not part of the last
* "batch" set are considered candidates for removal. Similarly, Arguments
* that are part of the "batch" set but not the "current" set are candidates
* for addition.
*
* The number of objects added and removed maybe limited with the `..Limit`
* parameters.
* The number of objects added and removed may be limited with the
* `...Limit` parameters.
*
* Finally, the batcher determines if the batching is complete if the
* "current" arguments match the "batch" arguments.
Expand Down Expand Up @@ -156,7 +151,7 @@ export class Batcher<T> {
*
* Returns an array of at most `limit` keys.
*/
private setDifference(a: IIndex<any>, b: IIndex<any>, limit: number) {
private setDifference(a: Record<string, any>, b: Record<string, any>, limit: number) {
const diff = [];
const aKeys = Object.keys(a);
for (let i = 0; i < aKeys.length && limit > 0; i++) {
Expand All @@ -172,7 +167,7 @@ export class Batcher<T> {
/**
* Returns true of objects `a` and `b` have exactly the same keys.
*/
private setHasSameKeys(a: IIndex<any>, b: IIndex<any>) {
private setHasSameKeys(a: Record<string, any>, b: Record<string, any>) {
const aKeys = Object.keys(a);
const bKeys = Object.keys(b);
if (aKeys.length !== bKeys.length) {
Expand Down
6 changes: 5 additions & 1 deletion packages/table/src/tableBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ const UPDATE_PROPS_KEYS: Array<keyof ITableBodyProps> = [
"selectedRegions",
];

const RESET_CELL_KEYS_BLACKLIST: Array<keyof ITableBodyProps> = [
/**
* We don't want to reset the batcher when this set of keys changes. Any other
* changes should reset the batcher's internal cache.
*/
const RESET_CELL_KEYS_BLACKLIST: Array<keyof ITableBodyProps> = [
"columnIndexEnd",
"columnIndexStart",
"rowIndexEnd",
Expand Down