Skip to content

Remove @emotion/styled from components #1368

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.

Sign up for GitHub

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

Closed
wants to merge 9 commits into from
Closed

Remove @emotion/styled from components #1368

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
140d4db
chore: replace emotion/styled with emotion/css
atanasster Dec 14, 2020
e3217f2
update fuction name for devtools
atanasster Dec 14, 2020
a42cda3
remove React import for forwardRef
atanasster Dec 14, 2020
e957024
Merge branch 'develop' into remove-emotion-styled
atanasster Dec 14, 2020
50c4e38
chore: remove @emotion/styled from components deps
atanasster Dec 14, 2020
4852c8f
fix: code simplification
atanasster Dec 14, 2020
e521455
Merge branch 'develop' into remove-emotion-styled
atanasster Dec 16, 2020
98ed236
Update packages/components/src/Box.js
atanasster Jan 5, 2021
021546b
Merge branch 'develop' into remove-emotion-styled
lachlanjc Feb 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
},
"dependencies": {
"@emotion/react": "^11.1.1",
"@emotion/styled": "^11.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

index.d.ts imports StyledComponent and we leak it in our public API.

import { StyledComponent } from '@emotion/styled'
export const Box: StyledComponent<BoxOwnProps, BoxProps>

We'll need an update to index.d.ts here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can update it for the sake of it, however it is imported by @emotion/react as a devDependency anyway:

https://github.com/emotion-js/emotion/blob/a89d4257b0246a1640a99f77838e5edad4ec4386/packages/react/package.json#L54

Copy link
Member

Choose a reason for hiding this comment

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

as a devDependency anyway

It's not installed in user's node_modules then.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure what to do about it - all types are dev dependencies and they are not part of the production bundles.

  1. We can add the type and pretty much duplicate StyledComponent, with all the risks duplicating code entails.
  2. We can add @emotion/styled as a devDependency (different from using it at run-time as previously).
  3. We can leave as is.

I took #3 but open to different opinions.

Copy link
Member

@hasparus hasparus Jan 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We can add @emotion/styled as a devDependency (different from using it at run-time as previously).
Not sure what to do about it - all types are dev dependencies and they are not part of the production bundles.

"dependencies" and "devDependencies" have no inherent meaning in the context of bundling. If I remember correctly, brunch bundler used to bundle all "dependencies", but webpack doesn't care about them at all (It traverses your module tree.)

They have meaning for package managers however. If a package A has package B in "dependencies", both will appear in user's node_modules when they do npm install A. This is not the case for "devDependencies". In case of "peerDependencies", the user will get a warning in the terminal prompting them to install the peerDependency.

If we use types from a devDependency, users won't know to install the package, but the types will collapse to any or fail their build when their tsconfig option skipLibCheck is set to false.

You can read more about it in npm docs:

There are also bundledDependencies and optionalDependencies, but I haven't seen anybody use them, so I'm not sure what's the support like.

Copy link
Member

Choose a reason for hiding this comment

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

What part of the type do we need from Emotion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its just ‘StyledComponent’

theme-ui/packages/components/index.d.ts

Line 2 in 90b7c10

import { StyledComponent } from '@emotion/styled'

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the review, I think it should be a devDependency.

So if somebody is depending just on theme-ui, and doesn't have @emotion/styled in their own dependencies or devDependencies, this change will remove @emotion/styled from their node_modules.

If they use TypeScript with skipLibCheck: false, we'll crash their build.
If they use JavaScript or skip lib check, we'll de facto disable editor support on Box and Flex, what is still an regression.

If we remove @emotion/styled from runtime, we need to stop depending on their types, especially since the types for Box and Flex is this PR are now incorrect.

Copy link
Member

Choose a reason for hiding this comment

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

@hasparus Will your PR to move @emotion/styled to user/peer deps fix this?

Copy link
Member

Choose a reason for hiding this comment

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

@lachlanjc I started working on removing @emotion/styled on remove-emotion-styled-2 branch — take a look at develop...remove-emotion-styled-2.

I think we'll release peer-deps branch and @emotion/styled removal in the same version — both might be breaking changes.

"@styled-system/color": "^5.1.2",
"@styled-system/should-forward-prop": "^5.1.2",
"@styled-system/space": "^5.1.2",
"@theme-ui/css": "0.6.0-alpha.1",
"@types/styled-system": "^5.1.10"
Expand Down
68 changes: 46 additions & 22 deletions packages/components/src/Box.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,64 @@
import styled from '@emotion/styled'
/** @jsx jsx */
import { jsx, useTheme } from '@emotion/react'
import { forwardRef } from 'react'
import { css, get } from '@theme-ui/css'
import { createShouldForwardProp } from '@styled-system/should-forward-prop'
import space from '@styled-system/space'
import color from '@styled-system/color'

const boxSystemProps = [...space.propNames, ...color.propNames]

/**
* @internal
* @type {(prop: string) => boolean}
*/
export const __isBoxStyledSystemProp = (prop) => boxSystemProps.includes(prop)

const shouldForwardProp = createShouldForwardProp(boxSystemProps)

const sx = (props) => css(props.sx)(props.theme)
const base = (props) => css(props.__css)(props.theme)
const variant = ({ theme, variant, __themeKey = 'variants' }) =>
css(get(theme, __themeKey + '.' + variant, get(theme, variant)))
css(get(theme, __themeKey + '.' + variant, get(theme, variant)))(theme)

export const Box = styled('div', {
shouldForwardProp,
})(
{
boxSizing: 'border-box',
margin: 0,
minWidth: 0,
},
base,
variant,
space,
color,
sx,
(props) => props.css
)
const objToArray = (obj) =>
obj ? Object.keys(obj).map((key) => ({ [key]: obj[key] })) : []

Box.displayName = 'Box'
const mergeProps = (props, initial, ...args) => {
return args.reduce(
(acc, fn) => [...acc, ...objToArray(fn(props))],
objToArray(initial)
)
}
export const Box = forwardRef(function Box(props, ref) {
const theme = useTheme()
const {
variant: variantProp,
__themeKey = 'variants',
__css,
css: cssProp,
sx: sxProp,
as: Component = 'div',
...rest
} = props
const style = mergeProps(
{ theme, ...props },
{
boxSizing: 'border-box',
margin: 0,
minWidth: 0,
},
base,
variant,
space,
color,
sx,
(props) => props.css
)
boxSystemProps.forEach((name) => {
delete rest[name]
})
return <Component ref={ref} css={style} {...rest} />
})

Box.withComponent = (component) => ({ as, ...props }) => {
console.warn('[theme-ui] You’re using the `.withComponent` API on a Theme UI component. This API will be deprecated in the next version; pass the `as` prop instead.')
return <Box as={component} {...props} />
}
export default Box
106 changes: 1 addition & 105 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1173,13 +1173,6 @@
resolved "https://registry.yarnpkg.com/@emotion/hash/-/hash-0.8.0.tgz#bbbff68978fefdbe68ccb533bc8cbe1d1afb5413"
integrity sha512-kBJtf7PH6aWwZ6fka3zQ0p6SBYzx4fl1LoZXE2RrnYST9Xljm7WfKJrU4g/Xr3Beg72MLrp1AWNUmuYJTL7Cow==

"@emotion/is-prop-valid@^0.8.1":
version "0.8.8"
resolved "https://registry.yarnpkg.com/@emotion/is-prop-valid/-/is-prop-valid-0.8.8.tgz#db28b1c4368a259b60a97311d6a952d4fd01ac1a"
integrity sha512-u5WtneEAr5IDG2Wv65yhunPSMLIpuKsbuOktRojfrEiEvRyC85LgPMZI63cr7NUqT8ZIGdSVg8ZKGxIug4lXcA==
dependencies:
"@emotion/memoize" "0.7.4"

"@emotion/is-prop-valid@^1.0.0":
version "1.0.0"
resolved "https://registry.yarnpkg.com/@emotion/is-prop-valid/-/is-prop-valid-1.0.0.tgz#1dbe82e52a12c065d416a702e2d106e552cde5be"
Expand All @@ -1198,7 +1191,7 @@
specificity "^0.4.1"
stylis "^4.0.3"

"@emotion/memoize@0.7.4", "@emotion/memoize@^0.7.1", "@emotion/memoize@^0.7.4":
"@emotion/memoize@^0.7.4":
version "0.7.4"
resolved "https://registry.yarnpkg.com/@emotion/memoize/-/memoize-0.7.4.tgz#19bf0f5af19149111c40d98bb0cf82119f5d9eeb"
integrity sha512-Ja/Vfqe3HpuzRsG1oBtWTHk2PGZ7GR+2Vz5iYGelAw8dx32K0y7PjVuxK6z1nMpZOqAFsRUPCkK1YjJ56qJlgw==
Expand Down Expand Up @@ -2758,20 +2751,6 @@
dependencies:
"@sinonjs/commons" "^1.7.0"

"@styled-system/background@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/background/-/background-5.1.2.tgz#75c63d06b497ab372b70186c0bf608d62847a2ba"
integrity sha512-jtwH2C/U6ssuGSvwTN3ri/IyjdHb8W9X/g8Y0JLcrH02G+BW3OS8kZdHphF1/YyRklnrKrBT2ngwGUK6aqqV3A==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/border@^5.1.5":
version "5.1.5"
resolved "https://registry.yarnpkg.com/@styled-system/border/-/border-5.1.5.tgz#0493d4332d2b59b74bb0d57d08c73eb555761ba6"
integrity sha512-JvddhNrnhGigtzWRCVuAHepniyVi6hBlimxWDVAdcTuk7aRn9BYJUwfHslURtwYFsF5FoEs8Zmr1oZq2M1AP0A==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/color@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/color/-/color-5.1.2.tgz#b8d6b4af481faabe4abca1a60f8daa4ccc2d9f43"
Expand All @@ -2786,77 +2765,13 @@
dependencies:
object-assign "^4.1.1"

"@styled-system/css@^5.1.5":
version "5.1.5"
resolved "https://registry.yarnpkg.com/@styled-system/css/-/css-5.1.5.tgz#0460d5f3ff962fa649ea128ef58d9584f403bbbc"
integrity sha512-XkORZdS5kypzcBotAMPBoeckDs9aSZVkvrAlq5K3xP8IMAUek+x2O4NtwoSgkYkWWzVBu6DGdFZLR790QWGG+A==

"@styled-system/flexbox@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/flexbox/-/flexbox-5.1.2.tgz#077090f43f61c3852df63da24e4108087a8beecf"
integrity sha512-6hHV52+eUk654Y1J2v77B8iLeBNtc+SA3R4necsu2VVinSD7+XY5PCCEzBFaWs42dtOEDIa2lMrgL0YBC01mDQ==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/grid@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/grid/-/grid-5.1.2.tgz#7165049877732900b99cd00759679fbe45c6c573"
integrity sha512-K3YiV1KyHHzgdNuNlaw8oW2ktMuGga99o1e/NAfTEi5Zsa7JXxzwEnVSDSBdJC+z6R8WYTCYRQC6bkVFcvdTeg==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/layout@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/layout/-/layout-5.1.2.tgz#12d73e79887e10062f4dbbbc2067462eace42339"
integrity sha512-wUhkMBqSeacPFhoE9S6UF3fsMEKFv91gF4AdDWp0Aym1yeMPpqz9l9qS/6vjSsDPF7zOb5cOKC3tcKKOMuDCPw==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/position@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/position/-/position-5.1.2.tgz#56961266566836f57a24d8e8e33ce0c1adb59dd3"
integrity sha512-60IZfMXEOOZe3l1mCu6sj/2NAyUmES2kR9Kzp7s2D3P4qKsZWxD1Se1+wJvevb+1TP+ZMkGPEYYXRyU8M1aF5A==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/shadow@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/shadow/-/shadow-5.1.2.tgz#beddab28d7de03cd0177a87ac4ed3b3b6d9831fd"
integrity sha512-wqniqYb7XuZM7K7C0d1Euxc4eGtqEe/lvM0WjuAFsQVImiq6KGT7s7is+0bNI8O4Dwg27jyu4Lfqo/oIQXNzAg==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/should-forward-prop@^5.1.2":
version "5.1.5"
resolved "https://registry.yarnpkg.com/@styled-system/should-forward-prop/-/should-forward-prop-5.1.5.tgz#c392008c6ae14a6eb78bf1932733594f7f7e5c76"
integrity sha512-+rPRomgCGYnUIaFabDoOgpSDc4UUJ1KsmlnzcEp0tu5lFrBQKgZclSo18Z1URhaZm7a6agGtS5Xif7tuC2s52Q==
dependencies:
"@emotion/is-prop-valid" "^0.8.1"
"@emotion/memoize" "^0.7.1"
styled-system "^5.1.5"

"@styled-system/space@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/space/-/space-5.1.2.tgz#38925d2fa29a41c0eb20e65b7c3efb6e8efce953"
integrity sha512-+zzYpR8uvfhcAbaPXhH8QgDAV//flxqxSjHiS9cDFQQUSznXMQmxJegbhcdEF7/eNnJgHeIXv1jmny78kipgBA==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/typography@^5.1.2":
version "5.1.2"
resolved "https://registry.yarnpkg.com/@styled-system/typography/-/typography-5.1.2.tgz#65fb791c67d50cd2900d234583eaacdca8c134f7"
integrity sha512-BxbVUnN8N7hJ4aaPOd7wEsudeT7CxarR+2hns8XCX1zp0DFfbWw4xYa/olA0oQaqx7F1hzDg+eRaGzAJbF+jOg==
dependencies:
"@styled-system/core" "^5.1.2"

"@styled-system/variant@^5.1.5":
version "5.1.5"
resolved "https://registry.yarnpkg.com/@styled-system/variant/-/variant-5.1.5.tgz#8446d8aad06af3a4c723d717841df2dbe4ddeafd"
integrity sha512-Yn8hXAFoWIro8+Q5J8YJd/mP85Teiut3fsGVR9CAxwgNfIAiqlYxsk5iHU7VHJks/0KjL4ATSjmbtCDC/4l1qw==
dependencies:
"@styled-system/core" "^5.1.2"
"@styled-system/css" "^5.1.5"

"@szmarczak/http-timer@^1.1.2":
version "1.1.2"
resolved "https://registry.yarnpkg.com/@szmarczak/http-timer/-/http-timer-1.1.2.tgz#b1665e2c461a2cd92f4c1bbf50d5454de0d4b421"
Expand Down Expand Up @@ -17659,25 +17574,6 @@ [email protected], style-to-object@^0.3.0:
dependencies:
inline-style-parser "0.1.1"

styled-system@^5.1.5:
version "5.1.5"
resolved "https://registry.yarnpkg.com/styled-system/-/styled-system-5.1.5.tgz#e362d73e1dbb5641a2fd749a6eba1263dc85075e"
integrity sha512-7VoD0o2R3RKzOzPK0jYrVnS8iJdfkKsQJNiLRDjikOpQVqQHns/DXWaPZOH4tIKkhAT7I6wIsy9FWTWh2X3q+A==
dependencies:
"@styled-system/background" "^5.1.2"
"@styled-system/border" "^5.1.5"
"@styled-system/color" "^5.1.2"
"@styled-system/core" "^5.1.2"
"@styled-system/flexbox" "^5.1.2"
"@styled-system/grid" "^5.1.2"
"@styled-system/layout" "^5.1.2"
"@styled-system/position" "^5.1.2"
"@styled-system/shadow" "^5.1.2"
"@styled-system/space" "^5.1.2"
"@styled-system/typography" "^5.1.2"
"@styled-system/variant" "^5.1.5"
object-assign "^4.1.1"

stylehacks@^4.0.0:
version "4.0.3"
resolved "https://registry.yarnpkg.com/stylehacks/-/stylehacks-4.0.3.tgz#6718fcaf4d1e07d8a1318690881e8d96726a71d5"
Expand Down