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

ThemeContext missing for external components #1107

Closed
tolesdev opened this issue Dec 14, 2018 · 12 comments
Closed

ThemeContext missing for external components #1107

tolesdev opened this issue Dec 14, 2018 · 12 comments

Comments

@tolesdev
Copy link

External Component

  • emotion version: 10.0.2
  • react version: 16.4.2

I know extending semantic ui is gross, WIP ripping it out

import React from 'react';
import PropTypes from 'prop-types';
import styled from '@emotion/styled';
import { Container } from 'semantic-ui-react';

const PageTitleContainer = styled.div`
  background: ${props => props.theme.colors.whiteHex} !important;
  padding: ${props => props.theme.pageTitle.titleContainerPadding} !important;
  border-bottom: ${props => props.theme.pageTitle.pageTitleBorder(props)} !important;  

  h1 {
    font-size: ${props => props.theme.pageTitle.h1.fontSize} !important;
    font-weight: ${props => props.theme.pageTitle.h1.fontWeight} !important;
    margin: ${props => props.theme.pageTitle.h1.margin} !important;
    padding: ${props => props.theme.pageTitle.h1.padding} !important;
    color: ${props => props.theme.colors.lightBlueHex} !important;
  }
`;


const FlexContainer = styled(Container)`
  display:flex !important;
  justify-content: space-between;
`

const ChildContainer = styled.div`
  display:flex;
  justify-content: space-between;

  &> div {
    &:first-of-type {
      margin-left: 0;
    }
    margin-left:15px;
  }
`
export const PageTitle = props => (
  <PageTitleContainer>
    <FlexContainer>
      <h1>{props.title}</h1>
      <ChildContainer>{props.children}</ChildContainer>
    </FlexContainer>
  </PageTitleContainer>
);

PageTitle.propTypes = {
  title: PropTypes.string
};

export default PageTitle;

Consuming Application

  • emotion version: 10.0.2
  • react version: 16.6.3
ReactDOM.render(
    <ThemeProvider theme={theme}>
      <PageTitle title="test"></PageTitle>
    </ThemeProvider>,
    document.getElementById('root')
);

What happened:
image

Reproduction:

  • Create external component
  • Create external theme object
  • Import both into CRA v2
  • Create a local component in CRA
  • Use ThemeProvider to provide theme prop

At this point the local component of your application should pick up the ThemeContext while the imported component will have an empty object for theme.

Problem description:

The component from our external library does not get the ThemeContext from the ThemeProvider and falls back to the default empty object. This was working prior to upgrading to emotion 10.x.x and worked sporadically using 10.x.x

The theme is also imported from our component library and can be used normally with styled components that are creating within the consuming application but it will not apply to the components that are imported en tandem with the theme.

@tolesdev tolesdev changed the title Theme empty for imported components ThemeContext missing for external components Dec 14, 2018
@Andarist
Copy link
Member

Maybe your library is trying to use other Provider? Can't know for sure though without runnable reproduction.

@tolesdev
Copy link
Author

tolesdev commented Dec 15, 2018

Unfortunately I'm not sure of a way to do it on Codesandbox . I would have to have the ability to create multiple projects and link them. Is uploading source code with instructions frowned upon? I'm aware it's certainly not ideal, but I don't know how to reproduce with the online tools I'm familiar with.

As for the the library using another Provider -- wouldn't a nested ThemeProvider simply overwrite whatever theme was from the context(s) above it? I'm positive I'm not wrapping a ThemeProvider when exporting the component and only once in the consuming application.

@Andarist
Copy link
Member

While codesandbox is preferred because it's just so easy to use you can also prepare sample repositories and list steps to setup them.

As for the the library using another Provider -- wouldn't a nested ThemeProvider simply overwrite whatever theme was from the context(s) above it? I'm positive I'm not wrapping a ThemeProvider when exporting the component and only once in the consuming application.

Not necessarily. If they have different identities then they won't shadow each other.

@tolesdev
Copy link
Author

Thanks for the response, I have added a reproduction of the issue as well as instructions to a git repository.

@Andarist
Copy link
Member

https://github.com/btoles/emotion-issue/pull/1

The problem is that when you install in some_lib it received whole node_modules, later when you link it in the app it becomes a linked directory in app's node_modules but it still has its own nested node_modules. So when your linked lib searches for @emotion/core package (which creates ThemeContext) it finds it in its node_modules but app find its @emotion/core in its node_modules. So they are different, they have different identities and u cant mix them.

You'd have to figure out how to work around this the best way in your case, you could ie:

  • make @emotion/core a peerDep of your lib so it wont install it automatically
  • remove some_lib/node_modules/@emotion/core as postlink step, so it can find the ones being higher in the directory tree (in app's node_modules)
  • use webpack's alias feature to force @emotion/core to a single directory

@tolesdev
Copy link
Author

I appreciate the help in diagnosing the problem, unfortunately I was unable to get it working even with 2 of your 3 suggestions. We are using create-react-app at the moment and don't plan to eject so the third option isn't viable for us quite yet. I'm not sure what changed with version 10 but this was a non-issue in version 9 and we'll probably stick with that.

@Andarist
Copy link
Member

Hm, peerDep and removing as postlink step were just ideas and it seems they dont work because the symlink gets resolved and it doesnt want to traverse up to the "host" app node_modules.

U dont have to eject - u could use i.e. https://github.com/timarney/react-app-rewired . I understand that you might be hesitant to that though.

I'm not sure what changed with version 10 but this was a non-issue in version 9 and we'll probably stick with that.

Emotion@9 was using legacy context which has used strings to identify which context it should use, emotion@10 is using new context API which is identity based, basically what happens is similar to this:

const noop1 = () => {}
const noop2 = () => {}
noop1 === noop2 // false

React just cant match those contexts because they are created by 2 "different" copies of emotion in your setup.

The same thing would happen with any other context-based library (i.e. react-router).

@HiranmayaGundu
Copy link

Hey @Andarist could you elaborate on the webpack alias approach? Currently we have a project structure similar to the one above

|_ app
|  |_ webpack.config.js
|  |_  package.json 
|  |_ node_modules  
|
|_ library
|  |_ webpack.config.js
|  |_ node_modules
|  |_ package.json

In library/webpack.config.js I have added the following

resolve: {
    alias: {
      '@emotion/core': path.resolve('../app/node_modules/@emotion/core'),
      '@emotion/styled': path.resolve('../api/node_modules/@emotion/styled'),
      '@miq/fiber-ui': path.resolve('../app/node_modules/@miq/fiber-ui'),
      'emotion-theming': path.resolve('../app/node_modules/emotion-theming')
    }
  }

I use yarn link to use library inside the app.

@miq/fiber-ui seems to use @emotion/core internally.

Everything builds fine, but at runtime I get this error

Uncaught TypeError: Cannot read property 'length' of undefined
    at serializeStyles (serialize.esm.js:141)
    at css (index.js:932)
    at Module.keyframes (index.js:772)
    at Object.../amap-ui/node_modules/react-spinners/ClipLoader.js (index.js:14403)
    at __webpack_require__ (index.js:21)
    at Object.../amap-ui/node_modules/@miq/fiber-ui/dist/es/Dropdown/DropdownBox.js (index.js:2823)
    at __webpack_require__ (index.js:21)
    at Object.../amap-ui/node_modules/@miq/fiber-ui/dist/es/Dropdown/Dropdown.js (index.js:2621)
    at __webpack_require__ (index.js:21)
    at Object.../amap-ui/node_modules/@miq/fiber-ui/dist/es/Dropdown/index.js (index.js:3058)

I have not created a git repository to replicate as it seems similar to the issue above, but can try to in case it's required.

@tolesdev
Copy link
Author

tolesdev commented May 11, 2020

Hey @HiranmayaGundu,

I finally figured out my issue, which may be yours as well. Going back to what @Andarist said, the newer Context API is identity based. The solution was to export the ThemeProvider from our component library itself instead of trying to use emotion-theming directly in the consuming application.

Use the ThemeProvider from @miq/fiber-ui -- if it's an internal package then just update it to export your provider.

@HiranmayaGundu
Copy link

HiranmayaGundu commented May 12, 2020

Hey @Btoles!

Thanks, will try that out! 😀

So from my understanding, we can have only one version of @emotion/core in our node_modules right? In my case, both library and app would use @miq/fiber-ui. It is a peerDependecy in the library. So if I want to develop using yarn link I would still have to ensure that it resolves to module that is there in the app?

@HiranmayaGundu
Copy link

After doing some more digging today, this issue seems to be similar to this.

I solved it by the method suggested in this comment.

So in my case I had used alias in the app instead of library and it seems to have worked. Thanks guys!

@tolesdev
Copy link
Author

tolesdev commented May 27, 2020

Hey @HiranmayaGundu,

I finally figured out my issue, which may be yours as well. Going back to what @Andarist said, the newer Context API is identity based. The solution was to export the ThemeProvider from our component library itself instead of trying to use emotion-theming directly in the consuming application.

Use the ThemeProvider from @miq/fiber-ui -- if it's an internal package then just update it to export your provider.

Follow up, learned more.

The problem was indeed two versions of the theming library.

Required:

// package.json
    // preferred if publishing library
{
    "peerDependencies": {
        "emotion-theming": "x.x.x"
    }
}

Using rollup:

// rollup.config.js
export default {
    external: ['emotion-theming']
}

Using webpack for bundling a library:
This is untested, just filling out based on documentation below.
https://webpack.js.org/configuration/externals/#string

// webpack.config.js
module.exports = {
    externals: {
        'emotion-theming': 'commonjs2 emotion-theming'
    }
}

Using webpack for bundling an application:
This is untested, just filling out based on documentation below.
https://webpack.js.org/configuration/resolve/#resolvealias

// webpack.config.js
const path = require('path');

module.exports = {
  //...
  resolve: {
    alias: {
      // point to the common version of the theming library
      'emotion-theming': path.resolve(__dirname, 'node_modules/emotion-theming')
    }
  }
};

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

No branches or pull requests

3 participants