Setting prop via `.attrs()` is not making the prop on the final component optional

[AntonNiklasson]

Related to #4302, #4303

I've been trying to understand what's missing in the typing for .attrs(). In #4306 I added a failing test case that highlights the issue we're seeing.

But I also found a bug in the test code: the tests relevant to this logic are actually wrong. Here's the diff that fixes the test:

@@ -210,7 +210,7 @@ interface MyStyle extends StyledObject<object> {
   textTransform?: StyledObject<object>['textTransform'];
 }

-const DivWithRequiredProps = styled.div.attrs<{ foo?: number; bar: string }>({
+const DivWithRequiredProps = styled.div.attrs<{ foo: number; bar: string }>({
   foo: 42, // Providing required prop foo, which makes it optional
 })``;

@@ -221,7 +221,7 @@ const DivWithRequiredProps = styled.div.attrs<{ foo?: number; bar: string }>({
 // @ts-expect-error and bar is still required
 <DivWithRequiredProps />;

-const DivWithUnfulfilledRequiredProps = styled.div<{ foo?: number; bar: string }>``;
+const DivWithUnfulfilledRequiredProps = styled.div<{ foo: number; bar: string }>``;

 // same test as above but with a wrapped, typed component
 const RequiredPropsProvidedAsAttrs = styled(DivWithUnfulfilledRequiredProps).attrs({

When I make this change, it surfaces the issue I'm seeing in my initial test case. With proper typing, foo should be optional here, but it's not:

[AntonNiklasson]

I've been toying with an idea to fix this, but I'm really in deep waters here 😊

What I think is missing is a piece of logic that just takes the props that was actually passed in to attrs and makes them optional in the final thing:

Something like this:

type ConsiderAttrs<
  Props extends object,
  Attrs extends object | ((...args: any[]) => any),
> = Attrs extends (...args: any[]) => infer ReturnedAttrs
  ? Partial<ReturnedAttrs> & Omit<Props, keyof ReturnedAttrs>
  : Partial<Attrs> & Omit<Props, keyof Attrs>;

// ...

PrivateResolvedTarget extends KnownTarget
  ? ConsiderAttrs<
      Substitute<
        Substitute<OuterProps, React.ComponentPropsWithRef<PrivateResolvedTarget>>,
        Props
      >,
      PrivateAttrsArg
    >
  : PrivateMergedProps,

That partly fixes the issue, but doesn't fully support a generic type to attrs where only parts of it was actually passed, like this:

const Component = styled.div.attrs<{ foo: number; bar: string }>({
  foo: 42
})``;

// @ts-expect-error
<DivWithRequiredProps />;

This should error, but doesn't with my addition.

[Martinocom-Switcho]

It was also asked here and marked as solved but then reverted. This .attrs wrong typing is holding us from updating to 6.

[lelukas]

@Martinocom-Switcho what version are you currently using? I dont remember the last working one. Do I have to downgrade typescript as well?

I think they mentioned TS v5 as a requirement for the v6 version. I really don't remember, though. Anyway, can you help me? I've seen too much. I don't want it more. I want to rollback.

[Martinocom-Switcho]

@lelukas We are still rocking on the v5 version of Styled-Components because of the .attrs bug, and we are not planning to upgrade till solved. Still, we upgraded our React Native application to 0.74 that has TS 5+ and it's working without problems*.

*The only problem we saw is that you should not use the "dot notation components", because their props are not up-to-date with the last React Native props. So, for example, instead of using styled.View we need to use styled(View). That's not a big deal and it's actually safer, because whenever the RN props will change, the type will be correct immediately, without having to wait an official update from Styled-Components.

On some of my personal project, always with RN 74, I used the "unsafe" workaround (link) that just specifies explicitly "hey, that prop is optional", even if it's not. It's unsafe because if you will specify the source of an image as optional and actually forgot to assign it, your app (web/mobile) will throw an error, even if TS will not complaining about it. I think downgrading is no longer necessary and it works with last versions.

[dpolugic]

I made an attempt to write an .attrs type that covered all cases yesterday but ran into a few issues. The main issue was related to a limitation in TypeScript, namely that TS does not allow inferring only some generic parameters -- it's all or nothing. It's easy to miss this when using default values for type parameters since there's no error, but the types won't be inferred as expected.

Related links

Issue in TS repo: https://github.com/microsoft/TypeScript/issues/10571

Open PR from 2018 by weswigham implementing partial inference: https://github.com/microsoft/TypeScript/pull/26349

Newer open PR from 2023 by weswigham doing the same: https://github.com/microsoft/TypeScript/pull/54047

Inference in .attrs

For our case with .attrs, the reason we run into this is the fact that .attrs has two jobs:

1. (Optional) Allow caller to augment component props type
2. Allow caller to set values of component props

That means we can have two sources of prop types:

1. Explicit type argument with extra props
2. .attrs argument value (as literal object or function return type)

Here we run into the issue above, because specifying explicit props (1) means we cannot infer the argument type (2).

Side note: This limitation is present in the SC5 types as well -- passing an explicit type argument means that the .attrs arg value type is ignored.

Potential solutions

Don't allow changing prop types with attrs

This would mean a breaking API change, but I think it's the simplest solution. Instead of letting .attrs have these two jobs, we'll only use it for setting existing component props. Augmenting the props can be done with .withConfig or with some new method instead.

const Comp = styled.div<{ strA: string }>``;

// -- Using `.withConfig` to augment props --
// This works in SC5, but requires specifying all the props. It's at least type-checked.
const StyledComp1 = styled(Comp)
  .withConfig<{ strA: string; numB: number }>({})
  .attrs({ strA: "hello" })``;

// -- Using potential new `.augment` method to augment props --
// Could be appropriate if we want to avoid re-using `withConfig`.
// `.augment` would only exist to change the type and would not have any runtime behavior.
const StyledComp2 = styled(Comp)
  .augment<{ numB: string }>()
  .attrs({ strA: "hello 2" })``;

Match SC5 behavior

We could match the SC5 behavior. It would mean that we ignore the type of the .attrs value argument if a type argument was specified.

This obviously isn't ideal since the types are still broken sometimes, but it might be acceptable depending on how .attrs is used in actual codebases.

Something else?

This is just what I came up with when trying to make the types work, there are definitely other solutions that I haven't considered.

Notes

This isn't the only issue with the current .attrs type, but it's kind of a blocker to actually fixing it. It's relatively easy to make props optional if present in the .attrs argument, but not at the same time as accepting a type argument.

Also, even if people agree with the approach, I'm not sure how you'd like to actually release a breaking change like this. Feedback from maintainers would be appreciated. I can prepare a PR if you think it seems viable.

[davidkarlsson]

@dpolugic I would really love to see a PR that matches the 5.x behavior of attrs if you got something that does.

It seems like a lot of people are stuck on v5 because of this issue and the maintainer has already shown an interest in getting a fix for this in https://github.com/styled-components/styled-components/pull/4288#issuecomment-2102720764 so my guess is it would get merged if it solves the issues even if they're not perfect because of the TS limitation that you mentioned.