expose __unsafe_useEmotionCache from @emotion/react #2441

garronej · 2021-07-23T12:46:55Z

What:

Expose __unsafe_useEmotionCache() from @emotion/react

Why:

Needed for seamless integration of tss-react with @emotion/react.
tss-react will be the new makeStyles in @material-ui v5.

How:

I exposed a proxy to useContext(EmotionCacheContext)

Checklist:

Documentation N.A? Being an interoperability feature, I don't think it has to be documented. But does it?
Tests
Code complete
Changeset Semantically it should be a minor but tell me if you feel that it should be a patch, I will rebase.

Hi @Andarist,
I implemented makeStyles on top of @emotion/react as you discussed with @oliviertassinari.
It's tss-react.
In v5 of material-ui makeStyle will be dropped and tss-react will be mentioned in the v4 to v5 migration guide for the teams that don't want to make the switch to the styled API.
It would be great if I could have access to the cache, tss-react have just been released and it has already been requested that it supports custom cache.
Of course I could expose my own provider but it would be much better if tss-react could blend in transparently with @emotion/react.

changeset-bot · 2021-07-23T12:46:57Z

🦋 Changeset detected

Latest commit: ac6b36b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@emotion/react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codesandbox-ci · 2021-07-23T12:53:44Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ac6b36b:

Sandbox	Source
Emotion	Configuration

oliviertassinari · 2021-07-25T21:22:31Z

packages/react/src/index.js

+export {
+  withEmotionCache,
+  CacheProvider,
+  useEmotionCacheContext
+} from './context'


The naming here feels a bit strange:

Why do we have CacheProvider not mentioning "emotion" but withEmotionCache does?

Why do we have withEmotionCache not mentioning "context" but useEmotionCacheContext does?

Agreed 😞 I'm stupid for putting 'Context',
However withEmotionCache and CacheProvider were named this way already. It's in the diff only because prettier rearranged the statement because it was becoming too long for a single line.
I assume it is out of the question to introduce breaking change for fixing name inconsistency.
That in mind what would you see as a better name for the hook?

useCache or

useEmotionCache

I personally prefer useEmotionCache, it is more consistent with withEmotionCache with which it shares the same goal of providing access to the cache.

I have rebased to export useEmotionCache instead of useEmotionCacheContext

Ok, in any case, I'm not a maintainer of this GitHub repository

thieryw · 2021-07-28T08:39:24Z

It would be a goodness if this could be merged!

Andarist · 2021-07-28T08:54:05Z

@mitchellhamilton what are your thoughts about this? It definitely shouldn't be encouraged to use this because there are not a lot of use cases this allows and they are mainly related to generating string class names that break 0-config SSR. OTOH, it's already possible to get access to the cache leveraging the public withEmotionCache - it's only way more problematic to do so (which we could treat as intentional). In the vein of being pragmatic over purist, I'm inclined to consent this - I'm pondering though if it shouldn't be exported with some kind of __unsafe prefix.

garronej · 2021-07-28T09:48:27Z

Hi @Andarist,
Thank you for taking the time to have a look.
I agree with your views. It's a shame to sacrifice 0-config SSR but unfortunately there is no other ways.
I am encline to use the __unsafe prefix, I think it's a good idea, I rebased.

Jack-Works · 2021-07-28T09:51:58Z

packages/react/src/context.js

+export let __unsafe_useEmotionCache =
+  function __unsafe_useEmotionCache(): EmotionCache | null {
+    return useContext(EmotionCacheContext)
+  }
+


Suggested change

export let __unsafe_useEmotionCache =

function __unsafe_useEmotionCache(): EmotionCache | null {

return useContext(EmotionCacheContext)

}

export function __unsafe_useEmotionCache(): EmotionCache | null {

return useContext(EmotionCacheContext)

}

I know, this is what I would write but I am trying to respect the repo conventions.

I wouldn't really bother about the existing convention here. The main reason why withEmotionCache is defined like that is because of the fact that it might be reassigned a few lines below - this doesn't apply to the __unsafe_useEmotionCache

Ah yes I see!
However I rebased to do it like this:

Because it is meant to be imported like this:
import { __unsafe_useEmotionCache as useEmotionCache } from "@emotion/react";

Andarist · 2021-07-28T10:52:31Z

Thank you for taking the time to have a look.

No problem, I will still wait on @mitchellhamilton's opinion though, before moving this forward.

emmatown · 2021-07-28T23:30:26Z

I'm fine with exposing this as __unsafe_useEmotionCache

oliviertassinari · 2021-08-01T20:53:00Z

Seeing this export make me think about something else related to emotion's cache & React context.
In @material-ui/styled-engine, the package has a direct dependency on @emotion/cache so it can access createCache. However, it seems to come with a few downsides:

All the developers that use Material-UI with styled-components download @emotion/cache unless they use an npm package alias. Proof: install and check the lock file.
The package size reported by bundlephobia is harder to follow, it does show how the dependencies are deduplicated once installed. Proof: https://bundlephobia.com/package/@material-ui/styled-engine@5.0.0-beta.1

Would it make sense to reexport the API of the packages that @emotion/react depends on? Now, maybe it doesn't make sense. For instance, for Material-UI, we have talked about making @emotion/styled a direct dependency instead of a peer dependency as it has no singletons.
So I guess I wished there we were a single emotion package to install and use, not multiples.

garronej · 2021-08-05T16:09:16Z

I'm fine with exposing this as __unsafe_useEmotionCache

@Andarist Is there any remaining blocking for this PR to be merged?
I hope I don't came out as being too pushy 😣, I promised emotion cache support to tss-react users weeks ago and I am embarrassed not to have delivered yet.

Andarist · 2021-08-05T17:29:56Z

Sorry for not handling this yet. Gonna go through all pending PRs tomorrow and ship a new version

Jack-Works · 2021-08-06T02:59:37Z

I'm fine with exposing this as __unsafe_useEmotionCache

@Andarist Is there any remaining blocking for this PR to be merged?
I hope I don't came out as being too pushy 😣, I promised emotion cache support to tss-react users weeks ago and I am embarrassed not to have delivered yet.

Don't feel embarrassed! I do not so hurry for that issue to be resolved. Thanks for all of your work!

garronej · 2021-08-06T07:41:53Z

Sorry for not handling this yet. Gonna go through all pending PRs tomorrow and ship a new version

No need to apologize, I can't imagine the amount of work required to maintain a project as widely used as emotion.

Don't feel embarrassed! I do not so hurry for that issue to be resolved. Thanks for all of your work!

Thanks for your understanding 😊 You are welcome.

.changeset/forty-pumas-refuse.md

garronej · 2021-08-09T09:26:11Z

Thank you 😊

ciampo · 2021-08-10T17:07:56Z

Hey @garronej and @Andarist , thank you for working on this! Would it be possible to include __unsafe_useEmotionCache() in the the TypeScript types for the @emotion/react package?

Thank you!

garronej · 2021-08-10T17:13:59Z

Hey @garronej and @Andarist , thank you for working on this! Would it be possible to include __unsafe_useEmotionCache() in the the TypeScript types for the @emotion/react package?

Thank you!

Since the use of this feature is discouraged if not for interoperability, I doubt @Andarist will be encline to let the type definitions be added for it. But maybe I'm wrong?
What about using module augmentation to import it?

ciampo · 2021-08-10T17:21:56Z

Hey @garronej and @Andarist , thank you for working on this! Would it be possible to include __unsafe_useEmotionCache() in the the TypeScript types for the @emotion/react package?
Thank you!

Since the use of this feature is discouraged if not for interoperability, I doubt @Andarist will be encline to add the types definition for it.
What about using module augmentation to import it?

Sounds good, thank you for the advice!

oliviertassinari reviewed Jul 25, 2021

View reviewed changes

garronej force-pushed the expose_useEmotionCacheContext branch from 208ffa0 to 772ba3f Compare July 26, 2021 01:40

garronej changed the title ~~expose useEmotionCacheContext in @emotion/react~~ expose useEmotionCache from @emotion/react Jul 26, 2021

garronej force-pushed the expose_useEmotionCacheContext branch from 772ba3f to 4a1aa67 Compare July 28, 2021 09:45

garronej changed the title ~~expose useEmotionCache from @emotion/react~~ expose __unsafe_useEmotionCache from @emotion/react Jul 28, 2021

Jack-Works reviewed Jul 28, 2021

View reviewed changes

garronej added 2 commits July 28, 2021 12:40

expose __unsafe_useEmotionCache from @emotion/react

df3d53c

Upgrade Changesets

ae143e2

garronej force-pushed the expose_useEmotionCacheContext branch from 4a1aa67 to ae143e2 Compare July 28, 2021 10:42

Andarist reviewed Aug 8, 2021

View reviewed changes

.changeset/forty-pumas-refuse.md Outdated Show resolved Hide resolved

Update .changeset/forty-pumas-refuse.md

ac6b36b

Andarist merged commit 24557d9 into emotion-js:main Aug 8, 2021

github-actions bot mentioned this pull request Aug 8, 2021

Version Packages #2394

Merged

Andarist mentioned this pull request Aug 8, 2021

Export the EmotionCacheContext #2418

Closed

4 tasks

garronej mentioned this pull request Aug 9, 2021

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

Closed

ciampo mentioned this pull request Aug 10, 2021

Components: use the __unsafe_useEmotionCache in the useCx hook WordPress/gutenberg#33982

Merged

6 tasks

garronej mentioned this pull request Aug 27, 2021

[RFC] Add makeStyles back? mui/material-ui#26571

Closed

2 tasks

garronej mentioned this pull request Mar 16, 2022

classes styles don't override default when using @emotion/react mui/material-ui#28045

Closed

2 tasks

garronej mentioned this pull request Apr 1, 2022

[system] Use cx instead of clsx mui/material-ui#32067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

expose __unsafe_useEmotionCache from @emotion/react #2441

expose __unsafe_useEmotionCache from @emotion/react #2441

garronej commented Jul 23, 2021 •

edited

changeset-bot bot commented Jul 23, 2021 •

edited

codesandbox-ci bot commented Jul 23, 2021 •

edited

oliviertassinari Jul 25, 2021

garronej Jul 25, 2021 •

edited

garronej Jul 26, 2021

oliviertassinari Jul 26, 2021

thieryw commented Jul 28, 2021

Andarist commented Jul 28, 2021

garronej commented Jul 28, 2021

Jack-Works Jul 28, 2021

garronej Jul 28, 2021

Andarist Jul 28, 2021

garronej Jul 29, 2021 •

edited

Andarist commented Jul 28, 2021

emmatown commented Jul 28, 2021

oliviertassinari commented Aug 1, 2021

garronej commented Aug 5, 2021

Andarist commented Aug 5, 2021

Jack-Works commented Aug 6, 2021

garronej commented Aug 6, 2021

garronej commented Aug 9, 2021

ciampo commented Aug 10, 2021

garronej commented Aug 10, 2021 •

edited

ciampo commented Aug 10, 2021

expose __unsafe_useEmotionCache from @emotion/react #2441

expose __unsafe_useEmotionCache from @emotion/react #2441

Conversation

garronej commented Jul 23, 2021 • edited

changeset-bot bot commented Jul 23, 2021 • edited

🦋 Changeset detected

codesandbox-ci bot commented Jul 23, 2021 • edited

oliviertassinari Jul 25, 2021

Choose a reason for hiding this comment

garronej Jul 25, 2021 • edited

Choose a reason for hiding this comment

garronej Jul 26, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 26, 2021

Choose a reason for hiding this comment

thieryw commented Jul 28, 2021

Andarist commented Jul 28, 2021

garronej commented Jul 28, 2021

Jack-Works Jul 28, 2021

Choose a reason for hiding this comment

garronej Jul 28, 2021

Choose a reason for hiding this comment

Andarist Jul 28, 2021

Choose a reason for hiding this comment

garronej Jul 29, 2021 • edited

Choose a reason for hiding this comment

Andarist commented Jul 28, 2021

emmatown commented Jul 28, 2021

oliviertassinari commented Aug 1, 2021

garronej commented Aug 5, 2021

Andarist commented Aug 5, 2021

Jack-Works commented Aug 6, 2021

garronej commented Aug 6, 2021

garronej commented Aug 9, 2021

ciampo commented Aug 10, 2021

garronej commented Aug 10, 2021 • edited

ciampo commented Aug 10, 2021

garronej commented Jul 23, 2021 •

edited

changeset-bot bot commented Jul 23, 2021 •

edited

codesandbox-ci bot commented Jul 23, 2021 •

edited

garronej Jul 25, 2021 •

edited

garronej Jul 29, 2021 •

edited

garronej commented Aug 10, 2021 •

edited