-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Primitives #658
Emotion Primitives #658
Changes from 6 commits
ec93579
3404e55
f989ea1
a886b94
775917e
a06d8aa
281b64e
231e581
4694063
97f5ac6
8772f90
2b49c02
74492fd
879e5eb
7a0bdc8
21ce603
608134b
3059db8
9018019
01f25cf
43f823a
3f1e514
93f89af
fca98c6
989c582
3ccc32e
7ff2302
372a94e
ff42dda
2a915b3
d61b23e
a670594
290e250
21c98fb
6a40c31
d6f458f
3f665fc
34f1e42
dc918d7
d9eeac7
4a645c0
e22c152
3e332c1
747acb3
b27ceb8
0fa0929
49c8172
51eecab
e05a360
a0c8a79
801c69d
1632b26
9c0b724
6df7787
99434b7
be28620
841a01a
b010d10
00c5108
e5408dd
1e5519f
b67bab7
c7bab6b
e91ab38
633761f
0ffcb92
5f9b040
75c57c9
0c5eb89
8a84d7c
53b984d
779c8c0
b6d4c17
a912bff
b3b5e93
437a9ab
477b0e2
2cfe87a
9b1920a
c46233c
d8f9810
33d236f
67fd321
e3a2055
38a0243
d2fe4c4
04e4bb3
cbdbf0f
b97b54e
e41ce0f
2254dd2
79ba84f
d2d17a2
30349a7
6538b79
f8e6958
e572f96
feeda5f
bab8aec
5714031
eb4b4fa
aa41a29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{ | ||
"name": "emotion-primitives", | ||
"version": "1.0.0-beta.1", | ||
"main": "src/index.js", | ||
"license": "MIT", | ||
"dependencies": { | ||
"css-to-react-native": "^2.2.0", | ||
"prop-types": "^15.6.1", | ||
"react-primitives": "^0.5.1" | ||
}, | ||
"devDependencies": { | ||
"enzyme": "^3.3.0", | ||
"enzyme-adapter-react-16": "^1.1.1" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import { StyleSheet } from 'react-primitives' | ||
import transform from 'css-to-react-native' | ||
|
||
// Copied and edited from @emotion/css | ||
// Cannot use css from @emotion/css directly because of unnecessary checks, __emotion_styles, and registering interpolations in cache. | ||
function handleInterpolation(interpolation, couldBeSelectorInterpolation) { | ||
if (interpolation == null) { | ||
return '' | ||
} | ||
|
||
switch (typeof interpolation) { | ||
case 'boolean': | ||
return '' | ||
case 'function': | ||
return handleInterpolation.call( | ||
this, | ||
this === undefined ? interpolation() : interpolation(this.props), | ||
couldBeSelectorInterpolation | ||
) | ||
|
||
default: | ||
return interpolation | ||
} | ||
} | ||
|
||
// Return evaluated css string | ||
function createStyles(strings, ...interpolations) { | ||
let stringMode = true | ||
let styles = '' | ||
|
||
if (strings == null || strings.raw === undefined) { | ||
stringMode = false | ||
styles += handleInterpolation.call(this, strings, false) | ||
} else { | ||
styles += strings[0] | ||
} | ||
|
||
interpolations.forEach(function(interpolation, i) { | ||
styles += handleInterpolation.call( | ||
this, | ||
interpolation, | ||
styles.charCodeAt(styles.length - 1) === 46 | ||
) | ||
|
||
if (stringMode === true && strings[i + 1] !== undefined) { | ||
styles += strings[i + 1] | ||
} | ||
}, this) | ||
|
||
return styles | ||
} | ||
|
||
function convertStyles(str) { | ||
if (typeof str === 'string' && str.length === 0) return str | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha, sure! |
||
|
||
const styleObj = [] | ||
|
||
const parsedString = str.split(';') | ||
|
||
parsedString.forEach(style => { | ||
if (typeof style === 'string') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this if statement seems unnecessary, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok! Makes sense. |
||
// Get prop name and prop value | ||
const ar = style.split(': ') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we split here by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more precise! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isnt |
||
|
||
if (ar[0] && ar[1]) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. both need to be coerced to booleans, we could make this more explicit by checking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. Earlier I was checking if both were not undefined! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how those elements could be undefined? checking against |
||
styleObj.push([ar[0].trim(), ar[1].trim()]) | ||
} | ||
} | ||
}) | ||
|
||
return styleObj | ||
} | ||
|
||
export function convertToRNStyles(styles) { | ||
if (styles[0][0] !== undefined) { | ||
let arr = [] | ||
|
||
arr.push(styles[0][0]) | ||
let len = styles.length | ||
let i = 1 | ||
for (; i < len; i++) { | ||
arr.push(styles[i], styles[0][i]) | ||
} | ||
|
||
let css = createStyles.apply(this, arr) | ||
|
||
let parsedCSS = convertStyles(css) | ||
|
||
// Convert css styles to react native | ||
let rnStyles = Array.isArray(parsedCSS) ? transform(parsedCSS) : {} | ||
|
||
return [StyleSheet.create({ style: rnStyles }).style] | ||
} else if (styles[0] == null || styles[0].raw === undefined) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit picking: this if (a) {
return val
}
if (b) {
return val2
} |
||
return styles | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. while I prefer this style emotion is aiming for high perf so it would probably be better to rewrite this to a for loop or forEach to avoid double iteration with |
||
.filter(style => { | ||
if (typeof style === 'object') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
return Object.keys(style).length > 0 | ||
} | ||
return true | ||
}) | ||
.map(style => { | ||
if (typeof style === 'object') { | ||
return StyleSheet.create({ style }).style | ||
} | ||
|
||
return style | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import * as React from 'react' | ||
import reactPrimitives from 'react-primitives' | ||
import PropTypes from 'prop-types' | ||
|
||
import { getStyles } from './getStyles' | ||
import { convertToRNStyles } from './convertToRNStyles' | ||
|
||
const isValidPrimitive = primitive => | ||
['Text', 'View', 'Image'].indexOf(primitive) > -1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe hoist this constant array above the function to avoid unnecessary garbage collection on this, although probably modern engines should recognize it's constant and hoist it on their own, but it's hard to tell - so I would argue it's better to do that manually |
||
|
||
const getPrimitive = primitive => { | ||
if (typeof primitive === 'string' && isValidPrimitive(primitive)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe instead of doing this, we could change babel-plugin-emotion to not change the dot syntax if the first character is uppercase? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be we can but I am not sure that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I really like this idea though 🤔 |
||
return reactPrimitives[primitive] | ||
} else if (typeof primitive === 'string' && !isValidPrimitive(primitive)) { | ||
throw new Error( | ||
`Cannot style invalid primitive ${primitive}. Expected primitive to be one of ['Text', 'View', 'Image']` | ||
) | ||
} else if (typeof primitive === 'function') { | ||
return primitive | ||
} | ||
} | ||
|
||
function evalStyles(context, Comp, styles, styleOverrides) { | ||
// Assign static property so that the styles can be reused (like in withComponent) | ||
Comp.styles = convertToRNStyles.call(context, styles) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be called on each render so the styles assigned as statics on the component will change depending on the props. Maybe the styles array created at the start of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do that but, using this has an advantage. We can use reuse the converted styles like - const Text = emotion.text`color: red; padding: 10px;`
// After calling convertToRNStyles, styles get converted to RN
// like this -> [73]
// If we decide to assign the styles array then we won't be able to
// reuse the styles on another component because styles array
// would be like this [['color: red; padding: 10px;', raw: [...]]].
const Author = emotion.text`${Text.styles} position: absolute; right: 0;` There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is good when composing the styles like using |
||
|
||
return getStyles.call(context, Comp.styles, context.props, styleOverrides) | ||
} | ||
|
||
/** | ||
* Creates a function that renders the styles on multiple targets with same code. | ||
*/ | ||
export function createEmotionPrimitive(splitProps) { | ||
/* | ||
* Returns styled component | ||
*/ | ||
return function emotion(primitive, { displayName } = {}) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. passing |
||
return createStyledComponent | ||
|
||
/** | ||
* Create emotion styled component | ||
*/ | ||
function createStyledComponent() { | ||
let styles = [] | ||
|
||
styles.push.apply(styles, arguments) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this should be an equivalent of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. This here makes sure that we receive interpolation array when using string template literal or a valid object when used object style notation. But if you've an alternate approach to this, you can alter this 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure what do u mean, u basically spread could u explain how they might not be the same? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll test and see if it works (doesn't break other things) without using |
||
|
||
class Styled extends React.Component { | ||
static propTypes = { | ||
innerRef: PropTypes.func | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This prop type won't work with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea.. I'll fix this! |
||
} | ||
|
||
onRef = innerComponent => { | ||
this.innerComponent = innerComponent | ||
|
||
if (this.props.innerRef) { | ||
this.props.innerRef(innerComponent) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we support the new createRef api like this? Also, is there a reason to get the ref at all since it doesn't seem like it's used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, we can! I already had this thing in my mind. |
||
} | ||
|
||
render() { | ||
const { toForward, styleOverrides } = splitProps( | ||
primitive, | ||
this.props | ||
) | ||
|
||
const emotionStyles = evalStyles(this, Styled, styles, styleOverrides) | ||
|
||
return React.createElement( | ||
getPrimitive(primitive), | ||
{ | ||
...toForward, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we go that way then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any perf. cost to this ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to the first one - yeah, to the second one (children) there is not, u can disregard my comment, this one doesn't have to be attached to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I'll tweak this in next commits! |
||
ref: this.onRef, | ||
style: emotionStyles.length > 0 ? emotionStyles : null | ||
}, | ||
this.props.children || null | ||
) | ||
} | ||
} | ||
|
||
Styled.primitive = primitive | ||
|
||
Styled.withComponent = (newPrimitive, options = {}) => | ||
emotion(getPrimitive(newPrimitive), { | ||
...options | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is making a copy here needed? (im not sure, it might be) |
||
})(...Styled.styles) | ||
|
||
Object.assign( | ||
Styled, | ||
getStyledMetadata({ | ||
primitive, | ||
styles, | ||
displayName | ||
}) | ||
) | ||
|
||
return Styled | ||
} | ||
} | ||
} | ||
|
||
const getStyledMetadata = ({ primitive, styles, displayName }) => ({ | ||
styles: primitive.styles ? primitive.styles.concat(styles) : styles, | ||
primitive: primitive.primitive ? primitive.primitive : primitive, | ||
displayName: displayName || `emotion(${getDisplayName(primitive)})` | ||
}) | ||
|
||
const getDisplayName = primitive => | ||
typeof primitive === 'string' ? primitive : primitive.displayName || 'Styled' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you should also consider reading |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
function evaluateStyles(styles, props) { | ||
return styles.map(style => { | ||
if (typeof style === 'function') { | ||
return style(props) | ||
} | ||
|
||
return style | ||
}) | ||
} | ||
|
||
export function getStyles(styles, props, styleOverrides) { | ||
const emotionStyles = evaluateStyles(styles, props) | ||
|
||
if (props.style) { | ||
emotionStyles.push(props.style) | ||
} | ||
|
||
if (styleOverrides && Object.keys(styleOverrides).length > 0) { | ||
emotionStyles.push(styleOverrides) | ||
} | ||
|
||
return emotionStyles | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import reactPrimitives from 'react-primitives' | ||
|
||
import { createEmotionPrimitive } from './createEmotion' | ||
import { splitProps } from './splitProps' | ||
|
||
const primitives = ['Text', 'View', 'Image'] | ||
|
||
const assignPrimitives = styled => { | ||
Object.assign( | ||
styled, | ||
primitives.reduce((getters, alias) => { | ||
getters[alias.toLowerCase()] = styled(reactPrimitives[alias]) | ||
return getters | ||
}, {}) | ||
) | ||
|
||
Object.assign( | ||
styled, | ||
primitives.reduce((getters, alias) => { | ||
const tag = alias.toLowerCase() | ||
getters[alias] = styled[tag]() | ||
getters[alias].tag = reactPrimitives[alias] | ||
getters[alias].displayName = `emotion.${tag}` | ||
return getters | ||
}, {}) | ||
) | ||
|
||
return styled | ||
} | ||
|
||
const emotion = createEmotionPrimitive(splitProps) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will it be possible to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use case is - To split component props, style props and style overrides so that we can forward them without logging any warnings. |
||
|
||
// Validate primitives accessed using the emotion function directly like emotion.TEXT`` or emotion.VIEW`` | ||
const validate = target => { | ||
const handler = { | ||
get: (obj, prop) => { | ||
if (prop in obj) { | ||
return obj[prop] | ||
} else { | ||
throw new Error( | ||
`Cannot style invalid primitive ${prop}. Expected primitive to be one of ['Text', 'View', 'Image']` | ||
) | ||
} | ||
} | ||
} | ||
|
||
return new Proxy(target, handler) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
export default validate(assignPrimitives(emotion)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldnt arrow function be good enough here? passing
this
like that is a little bit surprising for the reader at the first momentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was to me too when I first referenced this with emotion/css. We can change this, no issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did this to save a few bytes so that Babel didn’t need to add
var _this = this
. It won’t make a big difference but it helps. I’m fine with changing it thoughThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if that's the case I'm fine with keeping it here too