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

chore(Image): replace withSafeTypeForAs #2372

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Feb 19, 2020

WAT? ☃️

Currently we have withSafeTypeForAs utility which partially solves polymorphic typings for as prop but it's too complex and creates over-complicated type definitions in dist.

This PR removes usage of withSafeTypeForAs in Image with simple typings. Approach is inspired by recent changes in emotion: emotion-js/emotion#1405 & react-polymorphic-box. It's also compatible React.forwardRef.

Test snippet 🛠

import * as React from 'react'
import { Image, Button } from '@fluentui/react'

const ImageExample = () => (
  <>
    {/* Wrong default props */}
    <Image src="public/images/wireframe/square-image.png" to="Foo" />
    <Image src="public/images/wireframe/square-image.png" type="Foo" />

    {/* Wrong elements */}
    <Image as="aa" src="public/images/wireframe/square-image.png" />
    <Image as="blockq" src="public/images/wireframe/square-image.png" />

    {/* Wrong attrs on els */}
    <Image as="button" src="public/images/wireframe/square-image.png" type="Foo" />
    <Image as="a" src="public/images/wireframe/square-image.png" href1="Foo" />
    <Image as="button" src="public/images/wireframe/square-image.png" href="Foo" />

    {/* Wrong attr on components */}
    <Image as={Button} src="public/images/wireframe/square-image.png" type="Foo" />
    <Image as={Button} src="public/images/wireframe/square-image.png" typ="Foo" />
  </>
)

export default ImageExample

Fixes #1527

@layershifter layershifter changed the title proto: use type safe "as" chore(Image): replace withSafeTypeForAs Feb 19, 2020
@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Perf comparison

Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.47 0.38 1.24:1 2000 941
🦄 Button.Fluent 0.11 0.16 0.69:1 1000 107
🔧 Checkbox.Fluent 0.73 0.28 2.61:1 1000 734
🔧 Dialog.Fluent 0.32 0.16 2:1 5000 1609
🔧 Dropdown.Fluent 3.24 0.35 9.26:1 1000 3235
🔧 Icon.Fluent 0.12 0.03 4:1 5000 591
🎯 Image.Fluent 0.05 0.07 0.71:1 5000 230
🔧 Slider.Fluent 1.38 0.28 4.93:1 1000 1384
🔧 Text.Fluent 0.05 0.02 2.5:1 5000 251
🦄 Tooltip.Fluent 0.09 17.92 0.01:1 5000 442

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AccordionMinimalPerf.default 208 186 1.12:1
GridMinimalPerf.default 909 822 1.11:1
HeaderSlotsPerf.default 1365 1228 1.11:1
FormMinimalPerf.default 831 757 1.1:1
TreeWith60ListItems.default 229 209 1.1:1
TableMinimalPerf.default 632 587 1.08:1
Icon.Fluent 591 549 1.08:1
PortalMinimalPerf.default 230 218 1.06:1
SliderMinimalPerf.default 1480 1393 1.06:1
CustomToolbarPrototype.default 3415 3222 1.06:1
LayoutMinimalPerf.default 507 482 1.05:1
Image.Fluent 230 219 1.05:1
CarouselMinimalPerf.default 2178 2087 1.04:1
ProviderMinimalPerf.default 600 575 1.04:1
TooltipMinimalPerf.default 636 609 1.04:1
Dialog.Fluent 1609 1547 1.04:1
AttachmentMinimalPerf.default 893 869 1.03:1
RefMinimalPerf.default 155 151 1.03:1
TextMinimalPerf.default 282 274 1.03:1
VideoMinimalPerf.default 657 636 1.03:1
AlertMinimalPerf.default 551 542 1.02:1
ChatMinimalPerf.default 421 411 1.02:1
ListWith60ListItems.default 148 145 1.02:1
PopupMinimalPerf.default 307 300 1.02:1
TreeMinimalPerf.default 845 831 1.02:1
Avatar.Fluent 941 926 1.02:1
Tooltip.Fluent 442 435 1.02:1
AvatarMinimalPerf.default 536 530 1.01:1
DropdownMinimalPerf.default 3399 3367 1.01:1
RadioGroupMinimalPerf.default 388 386 1.01:1
Slider.Fluent 1384 1375 1.01:1
Text.Fluent 251 249 1.01:1
BoxMinimalPerf.default 231 231 1:1
DividerMinimalPerf.default 875 872 1:1
ItemLayoutMinimalPerf.default 1665 1667 1:1
ListCommonPerf.default 715 715 1:1
ListNestedPerf.default 665 667 1:1
MenuButtonMinimalPerf.default 1417 1419 1:1
ReactionMinimalPerf.default 2382 2393 1:1
Dropdown.Fluent 3235 3231 1:1
EmbedMinimalPerf.default 6188 6258 0.99:1
ListMinimalPerf.default 295 298 0.99:1
MenuMinimalPerf.default 1811 1827 0.99:1
IconMinimalPerf.default 284 291 0.98:1
LabelMinimalPerf.default 760 776 0.98:1
LoaderMinimalPerf.default 2277 2319 0.98:1
AnimationMinimalPerf.default 457 471 0.97:1
ChatDuplicateMessagesPerf.default 368 381 0.97:1
DialogMinimalPerf.default 1600 1644 0.97:1
InputMinimalPerf.default 892 917 0.97:1
SplitButtonMinimalPerf.default 11427 11773 0.97:1
CheckboxMinimalPerf.default 3674 3829 0.96:1
HeaderMinimalPerf.default 495 517 0.96:1
ProviderMergeThemesPerf.default 1036 1081 0.96:1
SegmentMinimalPerf.default 1189 1240 0.96:1
FlexMinimalPerf.default 356 374 0.95:1
HierarchicalTreeMinimalPerf.default 748 785 0.95:1
ImageMinimalPerf.default 224 235 0.95:1
StatusMinimalPerf.default 242 254 0.95:1
Checkbox.Fluent 734 769 0.95:1
ButtonSlotsPerf.default 582 628 0.93:1
TextAreaMinimalPerf.default 2825 3041 0.93:1
Button.Fluent 107 115 0.93:1
AttachmentSlotsPerf.default 3190 3482 0.92:1
DropdownManyItemsPerf.default 419 458 0.91:1
ToolbarMinimalPerf.default 709 788 0.9:1
ChatWithPopoverPerf.default 510 610 0.84:1
ButtonMinimalPerf.default 117 153 0.76:1

Generated by 🚫 dangerJS

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.

Type checks on as prop are not working properly
2 participants