Make StyledComponent polymorphic #1588
Conversation
this gets rid of a lot of warnings and wrong auto formatting
This makes it possible to type styled functional components and enables a workaround for lack of tagged templates support of Flow
🦋 Changeset is good to goLatest commit: 87e720a We got this. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
I tested this on my own (very large - ~1k styled components) application. I noticed an issue in the following case: // ↓↓↓ Flow error, it requires us to annotate its props
const Container = styled.div();
export default () => <Container />;// ↓↓↓ This works
type Props = { children: React$Node };
const Container = styled<Props>('div')();
export default () => <Container />;I'm not sure why it's asking me to annotate it, since If I change it to this, it doesn't require any annotation, so it looks like Flow detects // ↓↓↓ Flow error, it requires us to annotate its props
const Container = styled.div();
export default () => (
<>
<Container />
</>
);Do you think is it alright or there's something off? ps: here's a repro if you want to play with it: I opened an issue on the Flow repo: facebook/flow#8166 |
Codecov Report
|
FezVrasta
force-pushed
the
fix/flow-type-improvements
branch
from
14eb8a1 to
299b102
Compare
FezVrasta
force-pushed
the
fix/flow-type-improvements
branch
from
299b102 to
3288cf9
Compare
I'm not sure - I don't know Flow's philosophy. The fact that a fragment there changes the situation there is kinda wild for me 😱 Maybe this is something you could ask Flow maintainers? Not sure how often they respond nowadays though. |
|
I was able to reproduce the same issue with just native React types: I asked the same question on SO, maybe someone knows more: I would say the fact this happens with native React types means this PR is okay, and if a bug present, it's in Flow and not in my types. |
|
@nmote gave me a very clear explanation here facebook/flow#8166 (comment) So everything is correct, I'll fix the CI and we can then merge it. |
|
Sure, just let me know when this is ready. Thanks for working on this! |
|
hopefully CI should pass now, this lint rule that prevents semicolons is killing me... |
|
I would like to add a page in the docs where I explain how the Emotion Flow types work, any idea where would it make sense to put it? |
Isn't precommit hook just fixing this for you automatically?
I would say that the best place is just under TypeScript in the Tooling section |
|
@Andarist I added the new documentation page, feel free to merge the PR if you think it's okay. |
In the case I hit, Prettier needed to run several times to properly format the code ( |
# Conflicts: # packages/styled-base/src/utils.js
There was a problem hiding this comment.
Seems OK for me, but would love @mitchellhamilton having a look at this too.
* chore: tell Prettier we are using Flow this gets rid of a lot of warnings and wrong auto formatting * fix: make CreateStyled callable function polymorphic This makes it possible to type styled functional components and enables a workaround for lack of tagged templates support of Flow * chore: update type annotation to use new style * fix: wrong formatting * style: fix prettier issues * chore: changeset * fix: just import the @emotion/sheet type * style: make prettier happy * docs: added Flow types documentation page * Update flow.mdx * refactor: rename P to Props # Conflicts: # packages/styled-base/src/utils.js
What:
implements/fixes #1587
Why:
read #1587
How:
StyledComponentis now polymorphic (just for theCreateStyledfunction)Checklist: