RFC: Performatic Component Model

[anitavincent]

RFC Performatic Component Model

Problem

Admin ui's components using the current component model are slow. While investigating table performance issues we found that rendering components using createComponent/useElement is less performatic than rendering native components (link to investigation report). The perfomance loss was impactful enough that it degraded the overall performance of the page when using admin-ui.

Why this matters?

The current components we provide are degrading the performance of our users pages, specially on pages with many components rendered and many re-renderings (like when using tables). Improving our architecture to be more performatic will improve responsiveness and drastically improve user experience on admin pages that use admin-ui.

Proposal

Stop using createComponent and useElement to create every component, rebuild components with alternative solutions to handle the points createComponent/useElement currently does.

Points handled by current component model:

• Parsing csx object
  useElement handles merging of base component style (our native styling) and users custom style added to csx prop.

• Injecting forward refs
  createComponent injects forward refs, witch is something we must do for all our components.

• Typing simplification
  createComponent simplifies the process of typing the component with a merge of native components forwarded props + additional props + default admin-ui props (like csx or as)

• Polymorphism
  when using useElement the resulting component always accepts the prop as and handles polymorphism.

New suggested architecture:

Removing the csx prop from all components, handle custom styles through classNames props, pre-parsed by the user with a csx function provided by us through a different package. Merge our native classnames and users classnames with cx. more on: #536

Handle ref forwarding on each components implementation with reacts forwardRef. Type components the traditional way. Not every component will have polymorphism (as prop) to reduce code complexity and improve performance.

Styling

Components will receive additional custom styling through className prop

import { Button, csx } from "@vtex/admin-ui";

const buttonStyle = csx({ backgroundColor: "$red30", width: 100 });

<Button className={buttonStyle}>
          Example
</Button>

Internally, base classNames will be built outside the component scope to improve performance, variant that affect the style will be forwarded as a data- attribute. Variants will be styled via css with data attribute selectors. An auxiliary function can be used to generate the selectors text to simplify implementation.

// button.css.ts
export const buttonStyle = csx({
  text: "$action1",
  border: "none",
  borderRadius: "$base",
  cursor: "pointer",
  position: "relative",
  // variant styling
  '[data-size="normal"]': {
    padding: "$space-2 $space-3",
    height: "2.25rem"
  },
  '[data-size="large"]': {
    padding: "$space-3 $space-4",
    height: "2.75rem"
  },
  '[data-size="normal"][data-bleed-y="true"]': {
    marginY: negative("$space-2")
  },
  '[data-size="large"][data-bleed-y="true"]': {
    marginY: negative("$space-3")
  }
});

// component.tsx
import * as style from "./button.css";

const Button (props) => {
  const {
      size = "normal",
      bleedY = false,
      bleedX = false,
      className = "",
      ...restProps
    } = props;
  
   return (
       <InternalButtonComponent
            {...restProps}
            className={csx(style.buttonStyle, className)}
            data-size={size}
            data-bleed-x={bleedX}
            data-bleed-y={bleedY}
      />
   )
}

Forward refs

Handle forward refs the "react way", inside each components main file.

export const Button = forwardRef(
  (props, ref) => {
      <div ref={ref} />
  }
)

Typing

Type the components props the vanilla way, extending the native/base components props and creating a separate type for each variant.

type ButtonSize = "normal" | "large";
type ButtonVariants =
  | "primary"
  | "secondary"
  | "tertiary"
  | "critical"
  | "criticalSecondary"
  | "criticalTertiary"
  | "neutralTertiary";

interface ButtonType extends ReakitButtonProps {
    // Variants
    size?: ButtonSize;
    variant?: ButtonVariants;
    
    // others
    loading?: boolean
    ...
}

Pros

• Faster rendering and more responsive pages
• Flexible for different css frameworks on the user side
• More control when creating new components
• Easier understanding / onboarding for new developers

Cons

• Breaking changes
• More things to add manually when starting a new component
• Styling: change on styling logic for developers when styling variant specific css. Since those styles will be on a data attribute selector developers will have to add specific selectors to overwrite them. Example:

Internal css:

export const buttonStyle = csx({
  text: "$action1",
  border: "none",
  borderRadius: "$base",
  cursor: "pointer",
  position: "relative",
  // since bg and color are in a selector you must add a selector to overwrite those
  '[data-variant="primary"]': {
      color: `action.${tone}.${variant}`,
      bg: `action.${tone}.${variant}`,
      ...
  }

user custom styling:

<Button
    className={csx({
      marginBottom: "40px",
      // bg prop will only work inside a selector, otherwise the selector on internal css would have higher priority
      '[data-variant="primary"]': {
        bg: "black"
      }
    })}
 >

Unresolved questions

• Polymorphism implementation details?
• Function to simplify writting data attibute selector string?
• No tests comparing performance between models
• How do we decide when a component should be polymorphic?

Next steps

• Document new styling model
• Open RFC for layout components
• Start migrating components to new model

References

• Table perfomance issues investigation report: https://www.notion.so/vtexhandbook/Table-Performance-d85396c61a4f4b448b7a7e60b9c14ca1
• Styling model RFC: RFC: Styling model #536
• Styling and typing POC: https://codesandbox.io/s/lucid-carlos-pvrhw9?file=/src/App.tsx:661-835

[lucasaarcoverde]

About the type handle, doing like that will work for every component:

type ComponentProps = React.ComponentPropsWithRef<'{element}'> & ComponentOptions

Maybe we can check if this is the best approach or if we can improve this by making it easier to apply the elements props

[lucasaarcoverde]

interface ButtonType extends ReakitButtonProps {
    // Variants
    size?: ButtonSize;
    variant?: ButtonVariants;
    
    // others
    loading?: boolean
    ...
}

What do you think of avoiding using extends Ariakit...?

IMO all of Ariakit's necessary props should be embedded into the state prop and the rest should be custom created by us or extended from the native element (i.e React.ComponentPropsWithoutRef<'element'>)

[matheusps]

+1 Agreed

[matheusps]

I think that styling and component model are tightly coupled. What do you think about joining both discussions here?

[lucasaarcoverde]

Agreed! We can have a separate RFC to discuss the styling packages (stitches and vanilla, for example)

[matheusps]

• Function to simplify writing data attribute selector string?

It can be as simple as this: https://github.com/vtex/admin-ui/blob/f21ea46c0a3f38ed997a65bfc2ce82cc15cb0ced/packages/admin-ui/src/modal/util.ts#L9

[matheusps]

BTW, the Modal is using this model. We can learn from it.

[lucasaarcoverde]

How do we decide when a component should be polymorphic?

Does still make sense to have the polymorphism as it was before? (<Box as="span">...</Box>, for example). I mean, the main advantage of having this polymorphism was that we could reuse the csx property within any HTML or custom component element, now with the className proposal I don't think it makes much sense

So my proposal is to have a polymorphism only for specific behaviors. (<Button as="a">...</Button>, for example). What do you think?

[anitavincent]

I think we'll not have Box anymore

[matheusps]

The RFC is approved by the team

[davicostalf]

@matheusps Can we close this? Have we already done all the changes we intended?