Switch from Emotion to Styled Components #631
Conversation
joe-bell
force-pushed
the
refactor/styled-components
branch
2 times, most recently
from
ab2cc09
to
3c9db2e
Compare
joe-bell
force-pushed
the
refactor/styled-components
branch
from
3c9db2e
to
8152302
Compare
.config/ts/tsconfig.base.json
Outdated
| @@ -13,6 +13,8 @@ | |||
| "baseUrl": "../", | |||
| "noUnusedLocals": true, | |||
| "noUnusedParameters": true, | |||
| // @TODO Consider downgrading styled-components types instead | |||
| "skipLibCheck": true, | |||
There was a problem hiding this comment.
Some more info here:
- [@types/styled-components] Dependency in @types/styled-components on @types/react-native causes issues with projects who depend on @types/node DefinitelyTyped/DefinitelyTyped#33015
- https://stackoverflow.com/questions/54706594/types-styled-components-duplicate-identifier-formdata
We could downgrade but it seemed like a good option for now:
https://stackoverflow.com/questions/52311779/usage-of-the-typescript-compiler-argument-skiplibcheck
There was a problem hiding this comment.
If this comes to a vote. I'd vote for downgrade or what the guys have come up with in this PR:
https://github.com/sinnerschrader/feature-hub/pull/484/files#diff-0b57ba9f62edd23ad6769957a9249f2dR1
As we're using Yarn, we could leverage this .yarnclean file for this purpose. Seems like a win-win. We get the fresh version and .yarnclean will clean up the irrelevant typings file for our purposes.
skipLibCheck will render interfaces from @types packages unused. Effectively reducing our safety net :(
There was a problem hiding this comment.
of course if .yarnclean hackyhack will work. Other than that. I'd suggest to downgrade and keep it there until a fixed version is out
There was a problem hiding this comment.
Nice find, I'll give this a spin. Thanks mate!
There was a problem hiding this comment.
Consider this done! (it was excruciating)
1b58ee5
| @@ -1,4 +1,4 @@ | |||
| import { CSSObject } from '@emotion/core'; | |||
| import { CSSObject } from 'styled-components'; | |||
There was a problem hiding this comment.
I'd suggest we'd follow this convention for declaring interfaces as opposed of a newer way.
https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-d-ts.html
There was a problem hiding this comment.
like this one ->
packages/components/src/types/declarations/styled-components.d.ts
There was a problem hiding this comment.
So create a .d.ts and extend it? I feel like this might be out of scope of the migration tbh
There was a problem hiding this comment.
types.ts => types.d.ts would do for this iteration. Next guy in the file could tickle it further towards greatness.
There was a problem hiding this comment.
I'm gonna leave it for now, but let's look at overhauling types separately
You can add an issue here!
https://coingaming.atlassian.net/browse/SPO-4566
Packages available for testing under
0.95.1-beta.9Description
Ditch
emotionand move fully over tostyled-components(sorry @karl-kallavus)Why?
Specifically, because:
(Emotion v10 unusable with pure TypeScript emotion-js/emotion#1046)
Too often types broke on minor/patch version bumps
(Issues with Jest, Typescript and Emotion Babel Plugin emotion-js/emotion#687, toHaveStyleRule does not work with Emotion 10 emotion-js/emotion#949 (comment))
(Problems surrounding SSR injection of <style> and unreliability of :first-child selectors emotion-js/emotion#1178)
Bonus
Not sure if this was the same before but we can actually reference
CSSObjectkeys in types too:https://github.com/coingaming/sportsbet-design/blob/3c9db2ec6074c5fe0186fbff10a66ab6ef6fc09f/packages/objects/src/media-object/media-object.ts#L5
How Has This Been Tested?
Tests have been completely re-written
Screenshots
N/A
Types of changes
Checklist