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

v10: Drop usage of deprecated React types #2560

Merged
merged 3 commits into from
Nov 25, 2021

Conversation

eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Nov 22, 2021

What:

Fixes breakage with planned React 18 types e.g. https://github.com/DefinitelyTyped/DefinitelyTyped/runs/4279099932?check_suite_focus=true#step:6:2266

Why:

We plan to remove deprecated types in React 18. Since Emotion v10 peer dependencies will allow React 18, the types should not break.
Planned changes: DefinitelyTyped/DefinitelyTyped#46691
Current progress: DefinitelyTyped/DefinitelyTyped#56210

How:

Replace SFC with FC

Checklist:

  • [ ] Documentation
  • Tests
  • Code complete
  • Changeset

@eps1lon eps1lon marked this pull request as ready for review November 22, 2021 11:19
@Andarist
Copy link
Member

The FC type has been introduced in @types/react@16.7.2 and that has been released 3 years ago. It should be OK for us to release this but there is a small breaking change here - so I'm mentioning this for completeness.

I wonder - is this blocking MUI anyhow? The PR is targeting the v10 line - I will probably backport all (most?) React 18 work when it actually gets released as a stable version but nothing has been done towards this goal yet.

Taking up on the occasion - are you considering doing this as part of the React 18 types?

@eps1lon
Copy link
Contributor Author

eps1lon commented Nov 22, 2021

The FC type has been introduced in @types/react@16.7.2 and that has been released 3 years ago. It should be OK for us to release this but there is a small breaking change here - so I'm mentioning this for completeness.

Note that you unfortunately don't declare @types/react. However, your last v10 release was tested with @types/react@16.8 so you were already assuming existence of React.FC.

I wonder - is this blocking MUI anyhow? The PR is targeting the v10 line - I will probably backport all (most?)

It's blocking the DefinitelyTyped repo. Some types for Storybook related addons pull in Emotion V10.

Taking up on the occasion - are you considering doing this as part of the React 18 types?

There isn't enough information there to draw up an actual plan. What is the current behavior? What is the expected behavior? What are you doing right now? I would suggest opening a dedicate issue that goes into more detail why the current behavior is a problem and how we could remedy that. And what the implications are for existing code.

@Andarist
Copy link
Member

There isn't enough information there to draw up an actual plan. What is the current behavior? What is the expected behavior? What are you doing right now? I would suggest opening a dedicate issue that goes into more detail why the current behavior is a problem and how we could remedy that. And what the implications are for existing code.

For completeness of this offtopic discussion here - I've created a discussion in the DefinitelyTyped repo here: DefinitelyTyped/DefinitelyTyped#57377

@changeset-bot
Copy link

changeset-bot bot commented Nov 25, 2021

🦋 Changeset detected

Latest commit: 29f52ea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@emotion/core Minor
emotion-theming Minor
@emotion/styled Minor
@emotion/styled-base Minor

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

@Andarist Andarist merged commit b5a2661 into emotion-js:v10 Nov 25, 2021
@eps1lon eps1lon deleted the fix/v10/deprecated-types branch November 25, 2021 23:16
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

Successfully merging this pull request may close these issues.

None yet

2 participants