styled-components / styled-components-website

The styled-components website and documentation
https://styled-components.com
MIT License
611 stars 437 forks source link

Update v6 migration guide for shouldForwardProp #939

Closed bcole808 closed 8 months ago

bcole808 commented 10 months ago

In v5, prop forwarding behaved differently depending on if the target was an HTML element, or a React Component.

This PR updates the code example showing how to re-implement the default behavior that existed in v5 in a way that more closely matches the v5 behavior.

woodreamz commented 9 months ago

You're right, this section is misleading, your proposal is the real v5 behaviour according to the v5 source code:

const isTargetTag = isTag(elementToBeCreated); // ======> typeof elementToBeCreated === 'string'
  const computedProps = attrs !== props ? { ...props, ...attrs } : props;
  const propsForElement = {};

  // eslint-disable-next-line guard-for-in
  for (const key in computedProps) {
    if (key[0] === '$' || key === 'as') continue;
    else if (key === 'forwardedAs') {
      propsForElement.as = computedProps[key];
    } else if (
      shouldForwardProp
        ? shouldForwardProp(key, validAttr, elementToBeCreated)
        : isTargetTag
        ? validAttr(key) // ======> validAttr = isPropValid 
        : true
    ) {
      // Don't pass through non HTML tags through to HTML elements
      propsForElement[key] = computedProps[key];
    }
  }

And I don't know if we should do another PR or use this one but the section Passed props is also misleading because it describes the v5 behaviour, not the default v6. As shouldForwardProp is not provided anymore, props are now passed to any HTML elements or React components.

woodreamz commented 9 months ago

Also noticed shouldForwardProp is not documented on StyleSheetManager section.