Coder Social home page Coder Social logo

Comments (33)

chaance avatar chaance commented on April 25, 2024 4

If someone would like to update the emotion example to mirror this chakra example that would be awesome: #35

Emotion supports SSR with streaming and we should be using their native helpers instead of trying to hack our own solutions: https://emotion.sh/docs/ssr#renderstylestonodestream

I plan on knocking this out today 🙂

from examples.

tavoyne avatar tavoyne commented on April 25, 2024 2

I'm not sure to agree. The point of examples is to help newcomers, not to confuse them. You mentioned Next.js, note that it does have a basic example (which indeed is quite trivial) but has no advanced one. About the fact that it's off topic, I apologise. I thought this was about making the emotion example better.

from examples.

chaance avatar chaance commented on April 25, 2024 2

I've been meaning to do this but it fell to the bottom of my to-do list. I'll try to get back on it next week if no one else beats me to it 😅

from examples.

minervabot avatar minervabot commented on April 25, 2024 2

@RobinClowers Emotion assumes the head element is not controlled by React, but in Remix the entire document is controlled. This is a fundamental problem which does not really have a great solution.

We evaluated Remix and decided "no thanks" for this reason.

from examples.

3x071c avatar 3x071c commented on April 25, 2024 1

@PJColombo If you pass an empty dependency array to useEffect, it will only re-apply the styles and re-render the entire app when Document is (un)mounted. To prevent unnecessary re-renders (around 3 on every navigation to an error boundary), I have memorized the children of the Document component (wrapping its JSX tree in a useMemo hook, and memorizing components with React.memo). This will still however re-apply the styles immediately after the page is first mounted, causing a performance hiccup on page reloads. I wrote a custom useOnRemount hook (here) to deal with that problem (you’ll have to use a context at the very top level to store a Boolean flag indicating whether the first mount has occurred or not). I’m still working on stopping useEffect from executing three times in a row when the Document is re-mounted by Remix. Re-opening as the example remains a bad template for poorly performant apps and could be improved a lot.

from examples.

tavoyne avatar tavoyne commented on April 25, 2024 1

Thanks for clarifying that. Well, the only thing we have for now is a broken, contextless example. It could be wise to rename it emotion-advanced, and also to have an emotion-basic example. You mentioned Next.js, here's what you can read on emotion's website about it:

image

That kind of context is crucial for adoption, IMO.

Most people using emotion don't have a clue about how it works in the background.

from examples.

3x071c avatar 3x071c commented on April 25, 2024 1

@tavoyne The basic example you linked actually does extract and inline critical CSS using "advanced" SSR techniques (definitely more simple than what the emotion docs suggest though). This is the true basic example, which as you can see is so stupidly simple (just install the dependencies and use the library) that there's no reason for one (but if you feel like it, you can of course submit a PR). This "basic" approach comes with many downsides that are not mentioned in the emotion docs and are the reason why so many users and maintainers reach for the "advanced" approach instead (see these notes by the official MUI maintainer, the chakra-ui docs (a UI framework which uses emotion) etc.). I opened this in the hope of preventing more people from worsening their own app when getting chakra-ui/mui/emotion up and running with Remix, and I think providing a full example that promotes spec compliance and other problems from arising should be the default/go-to approach in the Remix examples.

from examples.

AndlerRL avatar AndlerRL commented on April 25, 2024 1

I've been playing with the example for a brief time and saw this error too.

First, Emotion on SSR (as styled-components) injects all the styles after reading the requested components so the server understand what to render and finish the job. This means that the styles that we create it's being send to the server and then to the browser.

By knowing that, every time that we assign emotionCache.sheet.container we are executing funcs under the hood, because Emotion is trying to read your new styles and therefore it has to change and execute under the hood, making the main emotionCache to trigger once again and, since we need that for reading emotionCache, I thought that a guard might work.

The only instance that this is happening, it's on the client useEffect at app/root.tsx, so we take that principle in advantage and we verify if the HTMLNodeElement has new nodes/children or new siblings inside of children or node like:

// Only executed on client
    useEffect(() => {
      // avoiding unwanted rendering
      if (document.head == emotionCache.sheet.container) return
// ...

At start it will take the first requested styles and, by testing this I haven't had problems with assigning new styles, removing styles/components removing nodes from function calls and going back/forward on the browser and styles are re-rendering whenever is necessary and performance issues were gone.

I hope to not to miss something?

from examples.

RobinClowers avatar RobinClowers commented on April 25, 2024 1

Just wanted to call out that this is still broken, and #78 does resolve the issue.

from examples.

PJColombo avatar PJColombo commented on April 25, 2024

Hey @3x071c, can you share here how did you solve this issue 🤔 ?.

Thanks!

from examples.

PJColombo avatar PJColombo commented on April 25, 2024

I see. Thank you for the explanation @3x071c. All these performance problems keep users from using the template and makes the styled-component app example a better CSS-in-js remix alternative in my opinion.

Let's keep the issue open until it get's solved or improved.

from examples.

tavoyne avatar tavoyne commented on April 25, 2024

Why is all the boilerplate in the emotion example even necessary? It's just working out of the box for me.

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@tavoyne Docs

from examples.

tavoyne avatar tavoyne commented on April 25, 2024

Yes thank you I've read the docs...

Is that suggesting that the point of this whole boilerplate is to allow nth child and similar selectors?

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@tavoyne Using emotion in the "out-of-the-box" SSR mode doesn't require an example, while the more advanced approach does. If one prefers the secondary approach for whatever reason (more control, better support etc.), I think they should get a proper example for that, just like Next.js (et al) do in their examples.

from examples.

machour avatar machour commented on April 25, 2024

@tavoyne We'd be more than glad to review a PR with the changes you've mentioned!

from examples.

tavoyne avatar tavoyne commented on April 25, 2024

@machour That's what I was planning to do. I just hoped that we could discuss things beforehand.

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@tavoyne I don't think an emotion-basic example is necessary, as there's no configuration needed whatsoever. That's why most often the advanced SSR use case is being showcased in examples, because it's way harder to get right. I also feel like your suggestion is a little off-topic to mine; I just opened this to call attention to the problem with the current inefficient emotion example that some might mistakenly copy into their own projects as-is.

from examples.

ancashoria avatar ancashoria commented on April 25, 2024

I agree with @tavoyne. I just wanted to see if remix works with emotion and the first thing I did was to look for this example which I consider is the ideomatic way of integrating Emotion with Remix. It has confused me for sure.

from examples.

jacob-ebey avatar jacob-ebey commented on April 25, 2024

If someone would like to update the emotion example to mirror this chakra example that would be awesome: #35

Emotion supports SSR with streaming and we should be using their native helpers instead of trying to hack our own solutions: https://emotion.sh/docs/ssr#renderstylestonodestream

from examples.

RobinClowers avatar RobinClowers commented on April 25, 2024

I ran into this same issue, everything seems to work fine without out this problematic code (including nth child selectors):

({ children, title }: DocumentProps, emotionCache) => {
const serverStyleData = useContext(ServerStyleContext);
const clientStyleData = useContext(ClientStyleContext);
// Only executed on client
useEffect(() => {
// re-link sheet container
emotionCache.sheet.container = document.head;
// re-inject tags
const tags = emotionCache.sheet.tags;
emotionCache.sheet.flush();
tags.forEach((tag) => {
(emotionCache.sheet as any)._insertTag(tag);
});
// reset cache to re-apply global styles
clientStyleData.reset();
}, [clientStyleData, emotionCache.sheet]);

The emotion SSR docs to show anything similar to this that currently, maybe they used to?

from examples.

ePirat avatar ePirat commented on April 25, 2024

@chaance Hi, did you manage to figure this out? I was just stumbling over this issue trying to use MUI with Remix and there are so many similar but not quite the same examples out there how to use Emotion with Remix, it's quite confusing 😕

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@ePirat @jacob-ebey @chaance Just tell Remix to not take over the entire document, but rather only a child of the body (like Next.js does). This will break <head>-specific functionality of remix (i.e. adding head tags through a special named export from each page), but react-helmet does a better job at this anyway. The default behavior of Remix is a terrible practice, React explicitly warns about it. Here's my fully working Remix + Emotion + Chakra UI website.

from examples.

machour avatar machour commented on April 25, 2024

@3x071c hey, where does React warns about hydrating the full document? Couldn't find that reference.

from examples.

minervabot avatar minervabot commented on April 25, 2024

clientStyleData.reset();
}, [clientStyleData, emotionCache.sheet]);

The present example is in fact, not correct. It causes a re-render loop by every time updating the context state in useEffect.

@3x071c I came here looking for whether Emotion works with Remix, and have found the answer to be no. The documentation is extremely negative about css-in-js, and the example is broken. MUI, for example, is pretty popular; it is short-sighted to tell its users to go away. Personally, I am scooting back to next.js where I came from.

@machour Not sure about official docs to the effect, but using document as the React container is not common. The existence and popularity of libraries like react-helmet at least strongly support the point. Pragmatically, having a <div> as the container is much easier. Often clients want to add their own weird tracking stuff (Google Tag Manager), or some overlaid gizmo in a <script> tag, and it is nice to have space for those things to work without royally messing up the React managed DOM. This is something I did not realize about Remix right away, and again, I am scooting right back to next.js where I came from.

from examples.

ePirat avatar ePirat commented on April 25, 2024

I've managed to make it work somehow by adapting the instructions from Chakra UI.

from examples.

jfsiii avatar jfsiii commented on April 25, 2024

We're using MUI w/Emotion in Remix following this example. It works with SSR (initial response is styled without/before JS) and after the page is hydrated

At one point we needed the node_compat flag for Cloudflare Workers, but that was resolved with emotion-js/emotion#2554

That Chakra UI example looks nearly identical and is easy to follow so I'd give that a shot

from examples.

machour avatar machour commented on April 25, 2024

Would be lovely if someone submitted a PR to the example 🙏🏼

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@minervabot This isn't a Next.js vs Remix thing, though it is a defaults/approach issue. It's not a Remix-specific issue, rather telling React to hydrate the entire document in general is a bad practice. Once you change the DOM root container setting up emotion (and anything based on it) is as simple as following the official emotion SSR docs. The project I linked above implements this successfully.

from examples.

3x071c avatar 3x071c commented on April 25, 2024

@3x071c hey, where does React warns about hydrating the full document? Couldn't find that reference.

@machour It used to be in the docs, can't find it anymore. But the warning is still thrown when you let React take over document.body: https://codepen.io/3x071c/pen/poKdKmP . This is why f.e. react-helmet doesn't take over the entire document to render <head> children, but instead instructs its users to inject it into the head (same goes for Next.js default behavior)

from examples.

RobinClowers avatar RobinClowers commented on April 25, 2024

I just opened #78 to fix this issue.

from examples.

minervabot avatar minervabot commented on April 25, 2024

The way the mui example handles the double-render paradox seems a lot better: https://github.com/mui/material-ui/blob/master/examples/remix-with-typescript/app/entry.server.tsx

from examples.

github-actions avatar github-actions commented on April 25, 2024

This issue has been automatically closed because we haven't received a response from the original author 🙈. This automation helps keep the issue tracker clean from issues that are unactionable. Please reach out if you have more information for us! 🙂

from examples.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.