Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

feat(context-selector): add initial implementation #2292

Merged
merged 5 commits into from
Feb 12, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Jan 30, 2020

This PR adds initial implementation of useContextSelector() and useContextSelectors() hooks.

What it solves?

Current API features shorthand collections:

<>
  <List
    items={[{ key: "one", content: "one" }, { key: "two", content: "two" }]}
  />
  {/* vs. 🥊 */}
  <List>
    <List.Item content="one" />
    <List.Item content="two" />
  </List>
</>

What is wrong with shorthand collections?

👎 it's too complicated to compose components and perform changes in React tree

<List
  items={[
    {
      key: "one",
      content: "one",
      children: (Component, props) => (
        <Tooltip>
          <Component {...props} />
        </Tooltip>
      )
    }
  ]}
/>;
{/* vs. 🥊 */}
<List>
  <Tooltip>
    <List.Item content="one" />
  </Tooltip>
</List>;

However, we need to propagate props somehow and React Context is only right thing. We cannot use React.Children as it will break on any wrapped element. But, React Context by default notifies all its consumers even if changed data are not relevant for that consumer 💣 And it cant' be solved by multiple React contexts:

<ListItemClick.Provider value={/* smth */}>
   {/* we will still receive updates there 💣 */}
  <ListActiveIndex.Provider value={/* smth */} />
</ListItemClick.Provider>

How it works?

  • stops context propagation via undocumented feature calculateChangedBits
  • implements Publish/Subscribe pattern to notify consumers and trigger updates based on selector's changes
// will receive all updates of `activeIndex` 👎
useContextSelector(Context, v => v.activeIndex);
// will receive updates on changes 👍
useContextSelector(Context, v => v.activeIndex === props.index);

Why two hooks?

Each useContextSelector() creates uses 3 hooks (useState(), useRef()) to represent its state and it's obviously bad for performance to use it to get 10 properties as it will create 30 hooks. useContextSelectors() will execute multiple selectors and will use only 3 hooks.

// 6 hooks 👎 
const cilcular = useContextSelector(Context, v => v.circular);
const debug = useContextSelector(Context, v => v.debug);
// vs.
// 3 hooks 👍 
const props = useContextSelectors(Context, {
  circular: v => v.circular,
  debug: v => v.debug
});

For PoC please check #2207 which contains changes in List component.

@layershifter layershifter force-pushed the feat/context-selectors branch from aacbb24 to 0bffe63 Compare January 30, 2020 13:19
@DustyTheBot
Copy link
Collaborator

DustyTheBot commented Jan 30, 2020

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.
⚠️ New package.json added: packages/react-context-selector/package.json. Make sure you have approval before merging!

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.47 0.37 1.27:1 2000 935
🦄 Button.Fluent 0.1 0.16 0.63:1 1000 104
🔧 Checkbox.Fluent 0.75 0.28 2.68:1 1000 749
🔧 Dialog.Fluent 0.31 0.16 1.94:1 5000 1560
🔧 Dropdown.Fluent 3.43 0.38 9.03:1 1000 3428
🔧 Icon.Fluent 0.13 0.04 3.25:1 5000 660
🦄 Image.Fluent 0.04 0.08 0.5:1 5000 215
🔧 Slider.Fluent 1.4 0.3 4.67:1 1000 1398
🔧 Text.Fluent 0.05 0.02 2.5:1 5000 262
🦄 Tooltip.Fluent 0.19 16.12 0.01:1 5000 966

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ListMinimalPerf.default 360 286 1.26:1
ChatDuplicateMessagesPerf.default 641 522 1.23:1
DividerMinimalPerf.default 1079 898 1.2:1
HierarchicalTreeMinimalPerf.default 1053 895 1.18:1
TextMinimalPerf.default 321 278 1.15:1
ImageMinimalPerf.default 257 228 1.13:1
ProviderMinimalPerf.default 752 674 1.12:1
Icon.Fluent 660 589 1.12:1
Tooltip.Fluent 966 863 1.12:1
TableMinimalPerf.default 678 620 1.09:1
CustomToolbarPrototype.default 4063 3738 1.09:1
TreeMinimalPerf.default 893 823 1.09:1
HeaderMinimalPerf.default 453 420 1.08:1
InputMinimalPerf.default 986 917 1.08:1
MenuMinimalPerf.default 2070 1924 1.08:1
RadioGroupMinimalPerf.default 411 380 1.08:1
PopupMinimalPerf.default 378 354 1.07:1
ProviderMergeThemesPerf.default 1184 1118 1.06:1
RefMinimalPerf.default 149 140 1.06:1
AttachmentMinimalPerf.default 962 914 1.05:1
AttachmentSlotsPerf.default 3314 3165 1.05:1
Text.Fluent 262 250 1.05:1
BoxMinimalPerf.default 229 220 1.04:1
ChatMinimalPerf.default 1701 1642 1.04:1
IconMinimalPerf.default 312 303 1.03:1
LayoutMinimalPerf.default 639 619 1.03:1
MenuButtonMinimalPerf.default 1573 1529 1.03:1
AnimationMinimalPerf.default 497 489 1.02:1
Avatar.Fluent 935 921 1.02:1
AvatarMinimalPerf.default 510 505 1.01:1
ButtonMinimalPerf.default 110 109 1.01:1
DropdownManyItemsPerf.default 490 484 1.01:1
LoaderMinimalPerf.default 2595 2562 1.01:1
ToolbarMinimalPerf.default 689 683 1.01:1
Dropdown.Fluent 3428 3402 1.01:1
EmbedMinimalPerf.default 6735 6707 1:1
TextAreaMinimalPerf.default 3438 3455 1:1
Checkbox.Fluent 749 748 1:1
CheckboxMinimalPerf.default 3819 3870 0.99:1
LabelMinimalPerf.default 896 906 0.99:1
PortalMinimalPerf.default 215 218 0.99:1
SliderMinimalPerf.default 1443 1451 0.99:1
AlertMinimalPerf.default 558 567 0.98:1
DropdownMinimalPerf.default 3579 3640 0.98:1
FlexMinimalPerf.default 352 361 0.98:1
ListCommonPerf.default 844 869 0.97:1
ReactionMinimalPerf.default 2477 2550 0.97:1
ChatWithPopoverPerf.default 562 586 0.96:1
DialogMinimalPerf.default 1671 1740 0.96:1
GridMinimalPerf.default 821 858 0.96:1
SegmentMinimalPerf.default 1202 1246 0.96:1
TooltipMinimalPerf.default 1113 1160 0.96:1
VideoMinimalPerf.default 659 690 0.96:1
Image.Fluent 215 223 0.96:1
Button.Fluent 104 109 0.95:1
Slider.Fluent 1398 1470 0.95:1
SplitButtonMinimalPerf.default 11393 12130 0.94:1
Dialog.Fluent 1560 1654 0.94:1
StatusMinimalPerf.default 264 285 0.93:1
FormMinimalPerf.default 778 843 0.92:1
HeaderSlotsPerf.default 1321 1436 0.92:1
ButtonSlotsPerf.default 548 619 0.89:1
ItemLayoutMinimalPerf.default 1710 1922 0.89:1
CarouselMinimalPerf.default 2085 2386 0.87:1
AccordionMinimalPerf.default 192 234 0.82:1

Generated by 🚫 dangerJS


## Usage

TODO
Copy link
Member Author

Choose a reason for hiding this comment

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

Oops

@layershifter layershifter merged commit f975942 into master Feb 12, 2020
@layershifter layershifter deleted the feat/context-selectors branch February 12, 2020 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants