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

[Emotion v10] Finish upgrading all things to emotion 10 #51

Merged
merged 2 commits into from
Feb 14, 2019

Conversation

snags88
Copy link
Contributor

@snags88 snags88 commented Feb 6, 2019

This PR adds the following to fully convert to emotion 10

  • Use babel-preset-css-prop
  • Remove babel-plugin-emotion
  • Update all css prop to not use string and align with new usage from emotion 10.
    • This includes docs, stories, and src files.
    • Also includes the svg -> react component template that SVGR uses.
  • Update jest configuration to use new jest-emotion config in serializer.
  • Update babel configurations to stop using emotion plugin and use the emotion css prop preset.
  • Update rollup config globals.

Copy link
Contributor

@ZeMunchkin ZeMunchkin left a comment

Choose a reason for hiding this comment

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

Looks good! Just a question on use of css from a stylesheet

@@ -82,7 +83,7 @@ class Alert extends React.Component {
>
<AlertContentContainer>
<Icon
css={[alertIconStyles]}
css={css`${alertIconStyles};`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify something? alertIconStyles is created in style.js using css. Is there a difference between creating it with css vs wrapping in css here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh nope, there's not. Both ways here are valid. https://emotion.sh/docs/composition

I just did a global search on css= and updated all of it to use the template literal.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

${selectorIcon};
color: ${checked ? COLORS.white : 'transparent'};
Copy link
Contributor

Choose a reason for hiding this comment

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

🙌 Good refactor on ordering

@snags88 snags88 merged commit 648e2e3 into master Feb 14, 2019
@snags88 snags88 deleted the update-jest-emotion branch February 14, 2019 20:38
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