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.

Sign up for GitHub

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

Jump to bottom

StyledComponent infer type from shorthand #3767

Closed
wants to merge 1 commit into from
Closed

StyledComponent infer type from shorthand #3767

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion definitions/npm/styled-components_v5.x.x/flow_v0.104.x-/styled-components_v5.x.x.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,10 @@ declare module 'styled-components' {
|};

declare export type StyledShorthandFactory<V> = {|
[[call]]: <StyleProps, Theme>(
[[call]]: (
string[]
) => StyledComponent<{ ... }, { ... }, V>,
Comment on lines +251 to +253
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for helping with this!

At first glance it seems okay. I would maybe put the more specific overload first, and this one after it.

Copy link
Contributor

@omninonsense omninonsense Mar 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think you could maybe add a special interpolation type that excludes functions, so people can still use interpolation inside the templates. Something like this:

    [[call]]: (
      string[],
      InterpolationBase[]
    ) => StyledComponent<{ ... }, { ... }, V>,

but from the top of my head, I am not sure if that will cause issues with inference, though

P.S: Maybe even accepting functions that don't accept any arguments could be added, too.

Copy link
Contributor Author

@penx penx Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@omninonsense sorry for the delay, I've tried to pick this up again but am getting a bit lost

I would maybe put the more specific overload first

if I put lines 251-253 lower down then the overload currently on line 254 is matched first and flow complains about "Missing type annotation for Theme, StyleProps:.

a special interpolation type that excludes functions

I think you meant to add a ...in? Like this?

    [[call]]: (
      string[],
      ...InterpolationBase[]
    ) => StyledComponent<{ ... }, { ... }, V>,

Also, for some reason I've tried various combinations of the above (including my original code) but am still getting this error:

Missing type annotation for V. V is a type parameter declared in function type [1] and was implicitly instantiated at
$ObjMap [2].

     417│     ElementName
     418│   >;
     419│
 [2] 420│   declare type ConvenientShorthands = $ObjMap<
     421│     BuiltinElementInstances,
 [1] 422│     <V>(V) => StyledShorthandFactory<V>
     423│   >;
     424│
     425│   declare interface Styled {
     426│     <Comp: React$ComponentType<P>, Theme, OwnProps = React$ElementConfig<Comp>>(

maybe you have time to check the branch out or for a quick call?

Copy link
Contributor Author

@penx penx Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this is recreating the error I'm seeing, will try to debug now:

https://flow.org/try/#0PTAEAEDMBsHsHcBQBLAtgB1gJwC6gEoCmAhgMZ6RayqgDkWJ5tA3IojgJ7qGgDKn0QgBNeAC2w5RxAHZCAYmRzYOAHgBqAPlABeUAAoAlDq0yOrTt1ABRAB44sisRKmydodRr1qj2rfw6CIuK4LvKKyh6s7Fw8AEIArsjQOMjSVoKohNI4AJLSAM44MqSE+W4A3oig1aBCyABuAFwEjDgAJOmEmdlEkCq0dfW0GogAvmwWPADCsNL1WchZOE4hMkJlum0A8gBGAFYAssToKlU1CUkpaRlLeYXFpQA0Z9W29o7BkmtnGlGks4VQIUAsJmjM5gslisvrINqBKgBIQbNQzGdyDUDAEbjRCEGyYXCgf4FPAHDgAEQabmBgQAdINWEA

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! On line 20 of the try-flow link, you forgot to call it (also tweaked the function signature to resemble a template literal, so flow doesn't complain about that then):

https://flow.org/try/#0PTAEAEDMBsHsHcBQBLAtgB1gJwC6gEoCmAhgMZ6RayqgDkWJ5tA3IojgJ7qGgDKn0QgBNeAC2w5RxAHZCAYmRzYOAHgBqAPlABeUAAoAzjizJpAcwDaAXQA0oAHSOZHawEodW5607dQAUQAPY0UxCSlZHVB1DT01d20tfg5BEXFccPlFZWjWdi4eACEAV2RoHFM-QVRCaRwASWkjGVJCA0iAb0RQbtAhZAA3AC4CRhwAEkrCatqiSBVaPv7aDUQAXzYfHgBhWGl+muQanFD0mSE23TGAeQAjACsAWWJ0FS6e4tLy6Unp+sacZqtGxvbqBYLkE6SM5vDS5Ui7IygIzJYTDHZ7A5HSEZC6gToASEWwz08S0KkWoGAK3WiEIAUwuFA8P+oAeHAAIgNIsiUvZFgADfmsIA

Copy link
Contributor

@omninonsense omninonsense Apr 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is still strange though, lmao.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to add a ...in? Like this?

Yeah, I forgot the spread

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 20 of the try-flow link, you forgot to call it

Cheers, though this solves the issue on this simplified example I still have the same error in my branch. As you say, the error seems strange. I don't think this is intended so I've raised a bug against Flow.

facebook/flow#8344

[[call]]: <StyleProps = { ... }, Theme = { ... }>(
string[],
...Interpolation<PropsWithTheme<StyleProps, Theme>>[]
) => StyledComponent<StyleProps, Theme, V>,
Expand Down