emotion-js / emotion

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

Is it possible to create a emotion cache in the Server mode on browser? #2691

Open Jack-Works opened 2 years ago

Jack-Works commented 2 years ago

Current behavior: Use emotion cache with @emotion/server cannot extract CSS on the browser.

To reproduce:

https://codesandbox.io/s/emotion-issue-template-forked-ij0lf7?file=/src/main.jsx

Expected behavior: I want to run SSR in browser

Environment information:

Andarist commented 2 years ago

Did you perhaps forget to save your codesandbox? It's not using @emotion/server anywhere as far as I can tell.

Jack-Works commented 2 years ago

Oh sorry, here is the new one. and I don't know why it does not work even I'm not using @emotion/server

https://codesandbox.io/s/emotion-issue-template-forked-1j1g0n?file=/src/main.jsx

the code sandbox link in the issue template is too old that use react-scripts v2 (latest is v5) and emotion 10. I have to manually upgrade them and everything breaks 😂

srmagura commented 2 years ago

Thanks for pointing that out @Jack-Works, I can fix the CodeSandbox soon. Created https://github.com/emotion-js/emotion/issues/2692.

Andarist commented 2 years ago

@Jack-Works CacheProvider is using value prop (just like all Providers created using React.createContext) and not cache prop to accept a cache. When I fix this I get this output:

Hello world!!

Is this matching your expectations? Or is this missing something?

Jack-Works commented 2 years ago

Hello world!!

As you can see, my style is not collected on the style sheet. the style tag above is empty.

css`
    color: red;
    font-size: 48px;
`
Andarist commented 2 years ago

This is an interesting use case that might be worth supporting. While this is an interesting use case - it's also rather a niche use case. For that reason, I would prefer to figure out a solution that wouldn't make us add too much code to the "common path" of Emotion.

I think the answer to that is to have a different code for @emotion/server when it is loaded in a browser. I've hacked around a few lines of code patching cache.insert to make it work without any changes elsewhere: https://codesandbox.io/s/emotion-issue-template-forked-2qetz0?file=/src/main.jsx

However, there is one caveat here for now... this doesn't work for global styles, only the scoped styles are handled. I'm not yet sure how to make global styles work but perhaps this is just a matter of some more thought.

There is also a question here though - do you really want to run this in a DOM environment? or just in a web worker or smth like this? Cause if the latter then we are working on introducing alternative builds for those envs and with those builds @emotion/server should work "out of the box" for the most part. The progress for this can be tracked here: https://github.com/preconstruct/preconstruct/pull/435

Jack-Works commented 2 years ago

Maybe createCache can receive the isBrowser from the options?

There is also a question here though - do you really want to run this in a DOM environment? or just in a web worker or smth like this?

Actually, I'm still thinking about it. We do have a use case to run SSR in the browser, but maybe I can move it into a Web Worker.

I think the answer to that is to have a different code for @emotion/server when it is loaded in a browser. I've hacked around a few lines of code patching cache.insert to make it work without any changes elsewhere: https://codesandbox.io/s/emotion-issue-template-forked-2qetz0?file=/src/main.jsx

Thanks for your hack! I'll try if that works for us! If it works well (or mostly well), I'm fine to use hacks to make it work.

Andarist commented 2 years ago

Maybe createCache can receive the isBrowser from the options?

It could but then we'd have to include all the related code in the "common path" and I'd like to avoid this.

Actually, I'm still thinking about it. We do have a use case to run SSR in the browser, but maybe I can move it into a Web Worker.

I still think there is merit in supporting this in a DOM environment - I've just mentioned that the web worker support might come sooner. The only blocker for the DOM environment right now is to figure out how to handle the global styles. I think that the rest should be fairly easy to implement within @emotion/server (using export conditions etc).

Thanks for your hack! I'll try if that works for us! If it works well (or mostly well), I'm fine to use hacks to make it work.

Let me know how it goes!

Andarist commented 2 years ago

I was also able to hack around Global issue: https://codesandbox.io/s/emotion-issue-template-forked-vec80x

This could be used together with a bundler. You could create a custom module that would look something like this:

import { Global as EmotionGlobal, useTheme, __unsafe_useEmotionCache  } from '@emotion/react'
import { serializeStyles  } from '@emotion/serialize'
export * from '@emotion/react'

function ServerGlobal({ styles, cache }) {
  let serialized = serializeStyles([styles], undefined, useTheme());
  let serializedNames = serialized.name;
  let serializedStyles = serialized.styles;
  let next = serialized.next;
  while (next !== undefined) {
    serializedNames += " " + next.name;
    serializedStyles += next.styles;
    next = next.next;
  }

  let shouldCache = cache.compat === true;

  cache.insert(
    ``,
    { name: serializedNames, styles: serializedStyles },
    cache.sheet,
    shouldCache
  );
  return null;
}

function Global(props) {
  const cache = __unsafe_useEmotionCache();
  if (cache.server) {
    return <ServerGlobal {...props} cache={cache} />;
  }

  return <EmotionGlobal {...props} />;
}

and alias @emotion/react to this custom module. This way everything would be "directly" imported from @emotion/react except Global. This would actually import this "wrapper" that would conditionally use different logic for your "server rendering".

I mostly present this as a workaround. I would like to figure out a better, builtin, solution - but at the moment I'm not sure how to best approach that.

Jack-Works commented 2 years ago

I'm still debugging this, it seems like @mui components do not use the cache I provided even I did as the mui document told 😂

Andarist commented 2 years ago

Do you perhaps have a repro case?

Jack-Works commented 2 years ago

Still debugging, not having a small repro. We have @mui and tss-react. Now I can get the CSS text from tss-react by your hack. @mui documentation said they use <CacheProvider> exported from @emotion/react but when I passed in, it does not use my own cache.

Jack-Works commented 2 years ago

image

interesting, if I add @mui in your hack example, it works

Jack-Works commented 2 years ago

Oh I know the reason, it's because we're using React 18!

Jack-Works commented 2 years ago

Same code, works in React 17 but not in 18

React 17: https://codesandbox.io/s/emotion-issue-template-forked-ih4od6?file=/src/main.jsx React 18: https://codesandbox.io/s/emotion-issue-template-forked-dn5dqr?file=/src/main.jsx

Andarist commented 2 years ago

Ah, yes - this is something that I've meant to mention here. That those presented workarounds only work in React 17 (well, the hack for Global should actually work everywhere) and that I need to figure out how to best handle this in React 18 (and if we figure this out then the same builtin solution would work also for React 17).

This is basically the same issue as with Global - the initial workaround for "scoped" styles doesn't work because Global injects styles from within useLayoutEffect. In React 18 we are using React.useInsertionEffect to inject "scoped" styles and thus Global and other stuff work in a similar way and thus the initial workaround doesn't work anymore with React 18 at all (because server-like rendering doesn't execute effects at all).

So really I see the only 2 options here to solve this in the Emotion (if we decide to fix it):

One thing that could potentially help you solve this right now:

This, in theory, should "just work" because you would use files that are capable of handling both the browser and the server logic in our render functions and we wouldn't resolve to browser-specific path because typeof document check that we are using would assume that it's not running in a browser environment and as such it should choose the "server path"

Jack-Works commented 2 years ago

Thanks for your help, I guess a worker will be the easiest way to do this. I'll try this tomorrow

jeffshaver commented 2 years ago

@Andarist we are running into the same issue (I think). we are essentially calling renderToStaticMarkup/renderToString from react-dom/server to generate html to provide to a library. in react 17, this works fine. in react 18 it doesn't at all.

when you say use patch package to remove package.json#broswer, can you help me understand what you mean more. I use patch-package but idk what I am meant to modify.

Andarist commented 2 years ago

I mean that you can try removing such entries from our package.json files: https://github.com/emotion-js/emotion/blob/2d3d7dd1d8b0de9dbfd50e42ee716ac00fb70ecd/packages/react/package.json#L6-L9

Please note that even if you do so you would still have to execute renderToStaticMarkup/renderToString in an environment without the global document object.

jeffshaver commented 2 years ago

thanks for the info!

this doesn't seem like a viable workaround for our use case.

the basic flow is that the library gives us a callback that we return html from. that callback happens synchronously. so I don't think we could move it into a worker.

are there any other options to solve this that wouldn't require moving the html generation into a worker?

jeffshaver commented 2 years ago

I think I tried basically everything on in the ssr page of the docs. renderStylesToString and what not. I tried making a cache and providing that to the cache provider.

jeffshaver commented 2 years ago

also, I know that react 18 just came out. so if we need to just kind of give it time, that's totally fine. just was looking into the issue I was having while trying to upgrade. I don't want to rush anyone unnecessarily. I don't NEED to upgrade to react 18 immediately.

Andarist commented 2 years ago

the basic flow is that the library gives us a callback that we return html from. that callback happens synchronously. so I don't think we could move it into a worker.

A synchronous XHR call to a worker? 🤣

On a more serious note, I would love to provide a proper solution for this BUT I think that we need to first implement support for new streaming APIs that are now available in React 18. This is a more pressing issue and one that might influence the solution for this issue here.

Unfortunately, I don't have any ETA for this streaming APIs work - we've talked about the implementation details for this a couple of months back with Mitchell but I never got enough time and mental capacity to start working on it. I plan to work on it... but I can't promise any deadlines.

jeffshaver commented 2 years ago

@Andarist thanks for the help! we will just wait on the upgrade for now. I haven't contributed to this project before, but if you think there are tasks that we could assist with in anyway I wouldn't be opposed to trying.

Jack-Works commented 2 years ago

One thing that could potentially help you solve this right now:

  1. use patch-package to remove package.json#browser from our packages
  2. move the "server-rendering" to a web worker

I did both, but it still cannot render @mui styles. I guess it still using useInsertionEffect to inject the style. I'll try to investigate this today.

Jack-Works commented 2 years ago

image

.inserted is empty. Still investigating...

Andarist commented 2 years ago

@Jack-Works do you have time for a quick call or something? we could potentially hash it out quickly

Jack-Works commented 2 years ago

image

image

I think I find it out! (But it does not match the source code in GitHub, maybe it because webpack did something)

@Jack-Works do you have time for a quick call or something? we could potentially hash it out quickly

Of course! How should I contact you?

Jack-Works commented 2 years ago

Ok, it is because I forgot to remove the "browser" field in the package.json of @emotion/styled. Now it works to get some CSS output, (but the render is still broken).

Andarist commented 2 years ago

@Jack-Works you can DM me on Twitter: https://twitter.com/AndaristRake

Jack-Works commented 2 years ago

I succeed. So the conclusion of this issue is to remove "browser" fields and it will work well in React 18.

jeffshaver commented 2 years ago

can we reopen this? I don't think that having a patch package workaround is a real solution here. or should I open a new issue? @Andarist what do you recommend

Andarist commented 2 years ago

I've reopened the issue

jeffshaver commented 2 years ago

@Andarist just following up on this. do we have any idea on a path forward? if so I don't mind trying to contribute the actual change. or if there is a possible workaround that would allow this to work in the browser (not in a worker).

thanks!

pushys commented 2 years ago

I seem to have a similar issue. Emotion (through MUI) stopped embedding style tags when used from renderToString or renderToStaticMarkup after upgrading to React 18. The only workaround I found is to render JSX manually using createRoot then all styles are embedded into the page again.

Andarist commented 2 years ago

I didn't have much time to think how I'd like to handle this. It is something that I'd like to get fixed, one way or another - but I can't promise any deadlines for this. While this is annoying I think there are some higher priority issues to be handled in relation to React 18 and I didn't have time to give my attention to them either.

The ideal solution here would solve this somehow without adding all of the "server-side" code to the default browser bundles (and that is a tricky part of this problem). I encourage any attempts to make it work and I promise to review PRs implementing this.

Jack-Works commented 2 years ago

Don't feel too stressed since there is a workaround! ❤️

jeffshaver commented 2 years ago

@pushys would you mind posting how you did it, cause the issue your mentioned is the exact issue I have (renderToStaticMarkup). i.e. do you cleanup as well?

pushys commented 2 years ago

@pushys would you mind posting how you did it, cause the issue your mentioned is the exact issue I have (renderToStaticMarkup). i.e. do you cleanup as well?

@jeffshaver, very hacky solution and not sure about its performance but for now I haven't come up with a more elegant way, unfortunately. The idea is to just render anything you want into a div and extract its inner HTML. Using it for rendering dynamic Leaflet icons.

import { useState, useLayoutEffect, useRef } from 'react';
import { createRoot, Root } from 'react-dom/client';
import { useMutationObserver } from 'rooks';

function useRenderToString(element: JSX.Element): string {
  const divRef = useRef<HTMLDivElement | null>(null);
  const rootRef = useRef<Root | null>(null);
  const [html, setHtml] = useState<string>('');

  useMutationObserver(
    divRef,
    () => {
      if (divRef.current?.innerHTML) {
        setHtml(divRef.current.innerHTML);
      }
    },
    { childList: true }
  );

  useLayoutEffect(() => {
    if (divRef.current === null) {
      divRef.current = document.createElement('div');
    }

    if (rootRef.current === null) {
      rootRef.current = createRoot(divRef.current);
    }

    rootRef.current.render(element);

    return () => {
      rootRef.current = null;
      divRef.current = null;
    };
  }, [element]);

  return html;
}
Jack-Works commented 2 years ago

I see a new minor version has been released. Is that solving this problem?

Andarist commented 2 years ago

Nope, it only allows for the server-like path to be automatically picked when bundling for a worker target. I didn't yet have a chance to look into fixing this issue here.

jisuo commented 2 years ago

@pushys would you mind posting how you did it, cause the issue your mentioned is the exact issue I have (renderToStaticMarkup). i.e. do you cleanup as well?

@jeffshaver, very hacky solution and not sure about its performance but for now I haven't come up with a more elegant way, unfortunately. The idea is to just render anything you want into a div and extract its inner HTML. Using it for rendering dynamic Leaflet icons.

import { useState, useLayoutEffect, useRef } from 'react';
import { createRoot, Root } from 'react-dom/client';
import { useMutationObserver } from 'rooks';

function useRenderToString(element: JSX.Element): string {
  const divRef = useRef<HTMLDivElement | null>(null);
  const rootRef = useRef<Root | null>(null);
  const [html, setHtml] = useState<string>('');

  useMutationObserver(
    divRef,
    () => {
      if (divRef.current?.innerHTML) {
        setHtml(divRef.current.innerHTML);
      }
    },
    { childList: true }
  );

  useLayoutEffect(() => {
    if (divRef.current === null) {
      divRef.current = document.createElement('div');
    }

    if (rootRef.current === null) {
      rootRef.current = createRoot(divRef.current);
    }

    rootRef.current.render(element);

    return () => {
      rootRef.current = null;
      divRef.current = null;
    };
  }, [element]);

  return html;
}

What kind of components does this support, only basic static components?

pushys commented 2 years ago

What kind of components does this support, only basic static components?

It should work with any JSX. For example, this is how I utilize it for Leaflet div icons:

const html = useRenderToString(<MarkerIcon color={color} size={size} />);

const divIcon = new L.DivIcon({
    html,
    className: '',
    iconSize: new L.Point(size, size),
});

It dynamically re-renders new HTML every time color or size prop changes. Unlike raw renderToString this hook is able to get emotion styles from MarkerIcon embedded into the page.

chandlervdw commented 1 year ago

Any update on this issue? Will Emotion support React 18 on SSR?

KrisKnez commented 1 year ago

This breaks production, any fixes?

Andarist commented 1 year ago

At the moment it's not possible to use the SSR API in the browser. You could try to patch-package things to make it work for your use case. I'd like to support this in the future but it doesn't have a high priority.

knutmarius commented 1 year ago

@pushys , we're facing a similar issue with Highcharts + MUI on React 18. Your useRenderToString seems to be a valid workaround approach, but how Highcharts works is that you can provide it with a formatter function that is called on various events, and you can return an HTML string from this function. This means we cannot instantiate the hook at base level of the parent React component, like you do. Do you have a suggestion to how this hook could be changed into a function that can be called directly within a synchronous function scope? I don't need the component to be dynamically updating based on prop changes., as Highcharts will just call the formatter function again if anything changes that would potentially call for a re-render...

KrisKnez commented 1 year ago

This is a simple solution that works with React 18

import { flushSync } from 'react-dom'
import { createRoot } from 'react-dom/client'

// This is a solution to this problem: https://github.com/emotion-js/emotion/issues/2691
// Did some basic testing and doesn't seem to degrade performance, some optimizations are most likely possible
// Maybe turn it into a hook that returns a render function and have tempEl and root only be initialized once?

const elementToString = (element: JSX.Element) => {
  const tempEl = document.createElement('div')
  const root = createRoot(tempEl)

  flushSync(() => {
    root.render(element)
  })

  return tempEl.innerHTML
}

export default elementToString

I am using this with Apexcharts' tooltip.

knutmarius commented 1 year ago

@KrisKnez : Thanks, that almost did the trick! However, it seems to fail on the initial renders of the chart. At that point I get this warning saying

flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

In that case the function returns just an empty string. If I then interact with the map, it suddenly renders everything OK. Have you experienced anything similar?

jmeiss commented 9 months ago

Is there any update regarding that issue? I'm facing the same bug on the first render than @knutmarius.