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

Read css and cx from import { CacheProvider } from '@emotion/react' #6

Closed
Jack-Works opened this issue Jul 23, 2021 · 21 comments
Closed

Comments

@Jack-Works
Copy link

Our code heavily depends on custom cache, and we have multiple emotion caches in the whole application. It makes it impossible to provide our custom cache when calling createMakeStyles

@Jack-Works
Copy link
Author

The current code requires the cache to be provided ahead-of-time instead of reading it from the CacheProvider https://github.com/garronej/tss-react/blob/main/src/createMakeStyles.tsx

@garronej
Copy link
Owner

garronej commented Jul 23, 2021

Ok, I understand, sorry for overseeing that.

@garronej
Copy link
Owner

garronej commented Jul 23, 2021

@Jack-Works, I submitted a PR to enable transparent support of custom cache.
Wether it get merged or not however I guaranty support of custom cache within a week.

@Jack-Works
Copy link
Author

Cool thank you!

@garronej
Copy link
Owner

The change have been approved. It should be merged soon.

@garronej
Copy link
Owner

garronej commented Aug 4, 2021

Sorry about the delay. I am releasing the feature as soon as the PR get merged on emotion

garronej added a commit that referenced this issue Aug 8, 2021
@garronej
Copy link
Owner

garronej commented Aug 8, 2021

@Jack-Works Everything should work now. Let me know if it isn't the case 😊
Doc

@Jack-Works
Copy link
Author

hi, @garronej I tried the latest version, it still not working and I got this:

You are loading @emotion/react when it is already loaded. Running multiple instances may cause problems. This can happen if multiple versions are used, or if multiple builds of the same version are used.

pnpm why @emotion/react -r
Legend: production dependency, optional only, dev only

Workspace Root

dependencies:
@emotion/react 11.4.1
@emotion/styled 11.3.0
└── @emotion/react 11.4.1 peer
@material-ui/core 5.0.0-alpha.34
├── @emotion/react 11.4.1 peer
├─┬ @emotion/styled 11.3.0 peer
│ └── @emotion/react 11.4.1 peer
└─┬ @material-ui/styled-engine 5.0.0-alpha.34
  ├── @emotion/react 11.4.1 peer
  └─┬ @emotion/styled 11.3.0 peer
    └── @emotion/react 11.4.1 peer
@material-ui/icons 5.0.0-alpha.34
└─┬ @material-ui/core 5.0.0-alpha.34 peer
  ├── @emotion/react 11.4.1 peer
  ├─┬ @emotion/styled 11.3.0 peer
  │ └── @emotion/react 11.4.1 peer
  └─┬ @material-ui/styled-engine 5.0.0-alpha.34
    ├── @emotion/react 11.4.1 peer
    └─┬ @emotion/styled 11.3.0 peer
      └── @emotion/react 11.4.1 peer
@material-ui/lab 5.0.0-alpha.34
└─┬ @material-ui/core 5.0.0-alpha.34 peer
  ├── @emotion/react 11.4.1 peer
  ├─┬ @emotion/styled 11.3.0 peer
  │ └── @emotion/react 11.4.1 peer
  └─┬ @material-ui/styled-engine 5.0.0-alpha.34
    ├── @emotion/react 11.4.1 peer
    └─┬ @emotion/styled 11.3.0 peer
      └── @emotion/react 11.4.1 peer

@masknet/theme packages\theme

dependencies:
tss-react 0.5.2
└── @emotion/react 11.4.1

I guess you should list @emotion/* packages as peer dependencies instead of direct dependencies?

@Jack-Works
Copy link
Author

Oh @material-ui/styled-engine are using 11.4.0 and tss-react using 11.4.1, let me try to fix this in our lockfile

@Jack-Works
Copy link
Author

Changed resolution to 11.4.1 but still getting this problem, I guess peer is required

@garronej
Copy link
Owner

garronej commented Aug 9, 2021

Ok,
Sorry for not testing this out with @material-ui/styled-engine

Changed resolution to 11.4.1 but still getting this problem, I guess peer is required

This is surprise me a lot.

As mentioned by @oliviertassinari here the problem is more complicated that it seems...

I will sort this out and come back with a solution.

@Jack-Works
Copy link
Author

Thanks! And I'm using pnpm workspace (that might affect the result)

@Jack-Works
Copy link
Author

Can you release an experimental version with peers so that I can test it by myself

@garronej
Copy link
Owner

garronej commented Aug 9, 2021

Done,

$ yarn add [email protected] @emotion/cache @emotion/react @emotion/serialize @emotion/utils

Sorry I was eager to push this feature out of the door but it wasn't ready yet.
Working on resolving the mui integrations issues.

@Jack-Works
Copy link
Author

Thanks! I have tried 0.6.0-beta.1, it fixes our problem!

@garronej garronej reopened this Aug 10, 2021
@garronej
Copy link
Owner

Thanks for reporting it working.
I really hate to make @emotion/cache, @emotion/react, @emotion/serialize, and @emotion/utils peer dependency though.
Problem being that @emotion is spitted into multiple little packages.
Using yarn packages are correctly deduped.
I would highly appreciate if you could give me the step to reproduce the error in your pnpm project so I can investigate the error and see if there isn't a better way than making everything a peer dependency.

@garronej
Copy link
Owner

Hi @Jack-Works,
In v0.6.0 there is a more exhaustive cache support.
The build is also much more carefully tested that the one I released in the hurry Sunday, in particular with the mui integration in an SSR context.
Let me know if you have any issue with it.
I've opted to leave only @emotion/react as dev dependency but I am still interested in reproducing the bug your experienced.
Best

@Jack-Works
Copy link
Author

Oh, I found there is still a problem here. Now tss-react works but mui styles are all breaking.

image

Both happening in 0.6.0 or 0.6.0-beta.1. Let me try to fix this locally

@Jack-Works
Copy link
Author

I've fixed this locally again by editing the lock file manually and playing around with node_modules. 😂

@garronej
Copy link
Owner

I'm glad you fixed it but it bothers me. Not everyone will have the skill to hack the lock file like you did.
I can't reproduce with yarn. Maybe there is just a simple keyword like resolve to put in the package.json that will make it work with pnpm.

@Jack-Works
Copy link
Author

I'm glad you fixed it but it bothers me. Not everyone will have the skill to hack the lock file like you did.
I can't reproduce with yarn. Maybe there is just a simple keyword like resolve to put in the package.json that will make it work with pnpm.

Thanks, but I can't tell how did I fix this exactally, our project is open sourced on GitHub. I guess you may be interested in it. DimensionDev/Maskbook@294b23a is the bad commit and in DimensionDev/Maskbook@ec225ca I fixed it by only changing package.json and pnpm-lock.yaml. But our project is hard to set up.

gitbook-com bot pushed a commit that referenced this issue Jan 22, 2022
gitbook-com bot pushed a commit that referenced this issue Jul 10, 2022
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

2 participants