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

perf(List): remove Flex usage#2025

Merged
layershifter merged 6 commits intomasterfrom
chore/remove-flex
Oct 18, 2019
Merged

perf(List): remove Flex usage#2025
layershifter merged 6 commits intomasterfrom
chore/remove-flex

Conversation

@layershifter
Copy link
Member

@layershifter layershifter commented Oct 14, 2019

⚠️ Do not use Flex in components in internals

Similar to #2023.

Any Stardust component will add additional style computations (merging, resolving) and will create performance degrade. One more reason to do not use Flexes: styles of Flex are not part of component so there is no way to generate static styles from if it used.

🚀 Measures

The test example below contains the worse scenario when all previously used Flexes will be rendered.

import * as _ from 'lodash'
import * as React from 'react'
import { List } from '@stardust-ui/react'

const items = _.times(100, i => ({
  key: i,
  media: 'Media',
  header: 'Foo',
  headerMedia: 'Bar',
  content: 'Baz',
  contentMedia: 'Qux',
}))

const ListPerf = () => <List items={items} />

export default ListPerf

Diff is around 39%. As ListItem is used in Dropdown, it should get performance benefits.

Before

Example Avg Median
List.perf.tsx 194.48 193.65

After

Example Avg Median
List.perf.tsx 131.42 130.44

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #2025 into master will increase coverage by 7.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2025      +/-   ##
==========================================
+ Coverage   68.52%   75.57%   +7.04%     
==========================================
  Files         910      160     -750     
  Lines        8306     5567    -2739     
  Branches     2438     1609     -829     
==========================================
- Hits         5692     4207    -1485     
+ Misses       2600     1346    -1254     
  Partials       14       14
Impacted Files Coverage Δ
packages/react/src/components/List/ListItem.tsx 100% <100%> (ø) ⬆️
packages/react/src/components/Flex/FlexItem.tsx 7.14% <0%> (-71.43%) ⬇️
packages/react/src/components/Flex/Flex.tsx 80% <0%> (-20%) ⬇️
...ckages/react/src/components/Popup/PopupContent.tsx 95.45% <0%> (-4.55%) ⬇️
...ges/react/src/components/Attachment/Attachment.tsx 87.5% <0%> (-3.81%) ⬇️
...es/react/src/components/Tooltip/TooltipContent.tsx 100% <0%> (ø) ⬆️
packages/react/src/index.ts 100% <0%> (ø) ⬆️
...s/components/RadioGroup/radioGroupItemVariables.ts
...mponents/Icon/svg/ProcessedIcons/icons-zoom-in.tsx
.../components/Icon/svg/ProcessedIcons/icons-hand.tsx
... and 762 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a715ab...f4bd1db. Read the comment docs.

@layershifter layershifter marked this pull request as ready for review October 16, 2019 11:35
@layershifter layershifter merged commit 5eba1ce into master Oct 18, 2019
@layershifter layershifter deleted the chore/remove-flex branch October 18, 2019 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants