Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interplay of shouldForwardProp clarifying question #4283

Open
Tbhesswebber opened this issue Mar 25, 2024 · 0 comments
Open

Interplay of shouldForwardProp clarifying question #4283

Tbhesswebber opened this issue Mar 25, 2024 · 0 comments

Comments

@Tbhesswebber
Copy link

I looked through the docs and searched the web, but didn't find anything helpful. In doing some exploration locally, I ended up digging through the codebase and found the answer, but it seems... unintuitive (maybe that isn't the right word)... when it comes to authoring a library. So, what is the "right" way to think about shouldForwardProp as it pertains to "inheritance" and is there a recommended way for library authors make things configurable without overloading consumers?

Below are some examples...

Let's set the theoretical stage using a contrived, but hopefully realistic example...

As a library author, my library is quite large, so the quickest path to upgrade is to leverage shouldForwardProp rather than migrate everything to transient props. I know that every component is going to need @emotion/is-prop-valid, so I opt for the StyleSheetManager route and wrap that into the library's top level element for theming.


One consumer reports that, StyleSheetManager#shouldForwardProp is intermittently working. They have additional global cases where they want control over forwarded props. In this case, last declaration "wins".

// this prevents their custom predicate from working
return (
  <StyleSheetManager shouldForwardProp={customShouldForwardProp}>
    <LibWrapper>
      {...}
	</LibWrapper>
  </StyleSheetManager>

// prevents my predicate from working
  <LibWrapper> 
    <StyleSheetManager shouldForwardProp={customShouldForwardProp}>
      {...}
    </StyleSheetManager>
  </LibWrapper>

I discover that my Input component is breaking because required is a valid attribute - now I update that component to leverage .withConfig to ignore that prop in particular. In the snippet below, global shouldForwardProp now, weirdly, no longer applies, which means I need to explicitly run my global checks locally. In this case, "last" declaration wins.

const Input = styled.div.withConfig({
  shouldForwardProp: (prop, component) => {
    if (prop === "required" && typeof component === "string") {
      return false;
    }
    return globalShouldForwardProp(prop, component);
  },
})``;

With the above change, a consumer decides that they want to forward the prop to the DOM for their use-case. Because I ignored it, their component never runs shouldForwardProp with that prop because I've shut it down. In this case, first declaration wins.

const MyInput = styled(Input).withConfig({
  shouldForwardProp: (prop, component) => {
    if (prop === "required" && typeof component === "string") {
      return true;
    }
    return globalShouldForwardProp(prop, component);
  },
})``;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant