Skip to content

v10: Drop usage of deprecated React types - #2560

Merged
Andarist merged 3 commits into
emotion-js:v10from
eps1lon:fix/v10/deprecated-types
Nov 25, 2021
Merged

v10: Drop usage of deprecated React types#2560
Andarist merged 3 commits into
emotion-js:v10from
eps1lon:fix/v10/deprecated-types

Conversation

@eps1lon

@eps1lon eps1lon commented Nov 22, 2021

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

eps1lon commented Nov 22, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

Comment thread .changeset/wicked-ducks-rest.md Outdated
Comment thread .changeset/wicked-ducks-rest.md Outdated
@changeset-bot

changeset-bot Bot commented Nov 25, 2021

Copy link
Copy Markdown

🦋 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.

2 participants