emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.5k stars 1.11k forks source link

Theming in react-native #1955

Open chochihim opened 4 years ago

chochihim commented 4 years ago

The problem In web, I can do following with css props:

import React from "react";
import { css } from "@emotion/core";

const myStyle = theme => css`
  background-color: ${theme.backgroundColor};
`;

const MyComponent = () => <div css={myStyle}>Foo</div>;

export default MyComponent;

The theme object is abstracted from the component.

However, on react-native I need to explicitly provide the theme object using useTheme:

import React from "react";
import { View, Text } from "react-native";
import { css } from "@emotion/native";
import { useTheme } from "emotion-theming";

const myStyle = (theme) => css`
  background-color: ${theme.backgroundColor};
`;

const MyComponent = () => {
  const theme = useTheme();
  return <View style={myStyle(theme)}><Text>Foo</Text></View>;
};

export default MyComponent;

I think it is more consistent if we also abstract the theme object on react-native.

Andarist commented 4 years ago

In the @emotion/native props.style are being concatenated with styles passed to a styled factory so this is treated as any interpolation and thus you can access theme by destructuring it from your props:

-const myStyle = (theme) => css`
+const myStyle = ({ theme }) => css`
  background-color: ${theme.backgroundColor};
`;

Whereas in the web version css prop is handled separately.

I agree that this doesn't look consistent though, so I will evaluate if we should change this in @emotion/native.

chochihim commented 4 years ago

In the @emotion/native props.style are being concatenated with styles passed to a styled factory so this is treated as any interpolation and thus you can access theme by destructuring it from your props:

-const myStyle = (theme) => css`
+const myStyle = ({ theme }) => css`
  background-color: ${theme.backgroundColor};
`;

Does this mean that I need to use styled API like this?

import styled, { css } from "@emotion/native";

const myStyle = ({theme}) => css`
  background-color: ${theme.backgroundColor};
`;

const MyVIew = styled.View`
  ${myStyle}
`;

const MyComponent = () => {
  return <MyVIew><Text>Foo</Text></MyVIew>;
};

But I want to stick with the "css prop" API. I wonder if we can have this in @emotion/native

Andarist commented 4 years ago

Does this mean that I need to use styled API like this?

No, you can do this right now:

const myStyle = ({theme}) => css`
  background-color: ${theme.backgroundColor};
`;

const MyVIew = styled.View``;

const MyComponent = () => {
  return <MyVIew style={myStyle}><Text>Foo</Text></MyVIew>;
};

But I want to stick with the "css prop" API. I wonder if we can have this in @emotion/native

This would be possible but has to be properly evaluated in terms of the API etc. I'll make sure to make a decision about this before shipping upcoming v11.

chochihim commented 4 years ago

Thank you for your suggestion! I give it a try and I guess I find a bug:

My code:

import React from 'react';
import {Text} from 'react-native';
import styled, {css} from '@emotion/native';

// css prop styles, theme.colors.primary.normal === '#21cedb'
const myStyleFunction = ({theme}) => css`
  color: ${theme.colors.primary.normal};
`;
const myStyle = css`
  color: #21cedb;
`;

// styled components
const StyledTextWithoutStyle = styled.Text``;
const StyledTextWithStyle = styled.Text`
  background-color: red;
  font-size: 20px;
`;

const MyComponent = () => {
  return (
    <>
      <Text css={myStyleFunction}>
        I wish this was possible in react-native because I hate naming a styled
        component{'\n'}
      </Text>
      <StyledTextWithoutStyle style={myStyleFunction}>
        This works as suggested!{'\n'}
      </StyledTextWithoutStyle>
      <StyledTextWithStyle style={myStyleFunction}>
        I expect this has red background and big font size but it does not. All
        the styles from {'<StyledTextWithStyle />'} is overrided{'\n'}
      </StyledTextWithStyle>
      <StyledTextWithStyle>
        Without the style, this has the styles from{' '}
        {'<StyledTextWithStyle />\n'}
      </StyledTextWithStyle>
      <StyledTextWithStyle style={myStyle}>
        Using the non-function style object, this also has styles from{' '}
        {'<StyledTextWithStyle />\n'} as well as the style object
      </StyledTextWithStyle>
    </>
  );
};

export default MyComponent;

And the result:

Did I miss anything? Btw I am using emotion@10.0.27

Also, you said you might change the API so that it is more consistent with the web version. I am not sure what the change is:

  1. Add css prop in react-native so that we can use something like <Text css={myStyleFunction} /> (i.e. without the styled API usage), or
  2. Allow following signature const myStyleFunction = (theme) => css`` so that we don't need to destructure the theme object from props.

If go for 2., it will still require the usage of styled to create a styled component, right? But it will be great if we can use the css prop API without styled API usage.

Andarist commented 4 years ago

Did I miss anything? Btw I am using emotion@10.0.27

Your code looks fine so not sure what's happening. I will try to look into this, but cant promise when - this shouldn't be hard to debug so if you could check this out I would highly appreciate it.

Add css prop in react-native so that we can use something like (i.e. without the styled API usage), or

Probably this.

chochihim commented 4 years ago

No problem, let me check it

chochihim commented 4 years ago

@Andarist Here's my finding after checking:

const myStyleFunction = ({theme}) => css`
  color: ${theme.colors.primary.normal};
`;

const StyledTextWithStyle = styled.Text`
  background-color: red;
  font-size: 20px;
`;

const MyComponent = () => (
  <StyledTextWithStyle style={myStyleFunction}>
    I expect this has red background and big font size but it does not. All
    the styles from {'<StyledTextWithStyle />'} is overrided{'\n'}
  </StyledTextWithStyle>
);

When rendering StyledTextWithStyle, there are two calls to the css function: one when rendering the styled component and another when interpolating myStyleFunction. The styling buffer is reset during the second call, and hence the first interpolated style is gone.

I am not sure how to fix it as I am not familiar with the codebase.

Andarist commented 4 years ago

@chochihim I would expect the same issue to happen with the buffer without using style prop at all. For example:

const StyledTextWithStyle = styled.Text`
  background-color: red;
  font-size: 20px;
  ${({ theme }) => css`
    color: ${theme.colors.primary.normal};
  `}
`;

Could you confirm this? That would be definitely worth solving.

A separate issue is the whole CSS prop-like API - it's a question if style should behave like the css prop? css prop is actually a "meta" prop on the web whereas style is not in both environments. OTOH on web we have both style & className and in React Native there is only style. I think that my conclusion is that we should disallow functions to be passed to style - this would match the web where className is the equivalent of RN's style. At the same time I would not introduce a special css prop-like API in here - that could be done later. My main motivation now is to avoid possible breaking changes in the future. WDYT?

chochihim commented 4 years ago

@Andarist Yes you're right. I can confirm it has the same problem using you example (I suppose you already confirmed it as you fixed the buffer thing)

And I agree with your other points. In fact I also didn't know (and expect) I can pass function directly to style prop until you suggested me to do so. It is much more natural to treat it like className in web.

And I definitely understand you want to leave breaking changes in the future. Thank you again for the help!

Andarist commented 4 years ago

We’ve merged in a PR disallowing functions as value of the the style prop, we wont be doing much beyond that though, so I’ve removed this from v11 milestone.

elramus commented 3 years ago

Hi, just checking that I understand where things stand with this currently. In React Native, the only way to incorporate your theme (outside of the component, anyway) is with the styled syntax? Getting ahold of theme using the css syntax is not supported?

Thanks for the awesome library 🙌