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

Move emotion packages and types for react-transition-group to peerDependencies #4972

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

uzikilon
Copy link

This PR is a suggestion to move emotion to "peerDependencies" avoid conflict with apps running other version of emotion

@changeset-bot
Copy link

changeset-bot bot commented Dec 29, 2021

🦋 Changeset detected

Latest commit: 2172f7f

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

This PR includes changesets to release 1 package
Name Type
react-select Patch

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 29, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2172f7f:

Sandbox Source
react-codesandboxer-example Configuration

@weedz
Copy link

weedz commented Jan 12, 2022

This will also close #4962 as it moves @types/react-transition-group to devDependencies 👍

@kauffmanes
Copy link

There's a bug in emotion that caused useInsertionEffect to be referenced directly in the specifiers list of the import statement. I was going to open a bug to update the version of emotion but I think this issue would take care of that problem as well?

emotion-js/emotion#2649
emotion-js/emotion#2651
https://stackoverflow.com/questions/71185635/material-ui-export-useinsertioneffect-imported-as-useinsertioneffect1-was

@dcousens dcousens changed the title Dependancies update Move emotion to a peerDependency Oct 19, 2022
@lukebennett88 lukebennett88 changed the title Move emotion to a peerDependency Move emotion packages and types for react-transition-group to peerDependencies Oct 25, 2022
lukebennett88
lukebennett88 previously approved these changes Oct 25, 2022
'react-select': patch
---

Move emotion packages and types for react-transition-group to peerDependencies
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it still patch if a dependency has been moved to peerDependencies?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Code that ships to the user has been changed, but it doesn't add any new functionality (minor) or change the existing API (major), so I'd consider it a patch (unless I'm missing something?)

Copy link
Collaborator

Choose a reason for hiding this comment

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

They might not already have emotion explicitly added as a dependency in their application, so wouldn't it be a potentially breaking change?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats a good point.
I think it varies by package manager, so it definitely could be a breaking change for a subset of users. I'll try to find someone who can give us a definitive answer 👍

This PR is a suggestion to move `emotion` to `"peerDependencies"`   avoid conflict with apps running other version of `emotion`
@lukebennett88 lukebennett88 added this to the 6.0 milestone Oct 25, 2022
@lukebennett88 lukebennett88 dismissed their stale review October 25, 2022 04:59

This PR introduces a breaking change

@lukebennett88
Copy link
Collaborator

Thanks so much for your PR @uzikilon.
Unfortunately this is a breaking change so we can't merge it until we're ready to do another major release.

@sorenhoyer
Copy link

In my opinion runtime css-in-js is the real problem here. Many have already started using zero-runtime css-in-js such as Linaria, and tbh I don't think the consumer who opted out of these messy libraries (emotion, styled-components etc.) should pay the price by having such a library installed and included in their code neither implicit nor explicitly,

So instead of asking the question whether it should be a peer- or normal dependency, perhaps there should be a PR for migrating away from using runtime css-in-js in the first place?

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Nov 8, 2022

See #5451 for discussion regarding decoupling from Emotion.

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

7 participants