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

@emotion/server crash on import in Worker environment #2446

Open
frandiox opened this issue Jul 29, 2021 · 22 comments
Open

@emotion/server crash on import in Worker environment #2446

frandiox opened this issue Jul 29, 2021 · 22 comments

Comments

@frandiox
Copy link

Current behavior:

Looks like @emotion/server always assumes a Node.js environment. However, it is now possible to do SSR in Workers environment (such as Cloudflare Workers). This package breaks on import because some dependencies (html-tokenize) try to access Buffer, Stream, and other Node-only APIs.

I've tried to get some of the problems fixed: LinusU/buffer-from#13
However, html-tokenize have other issues.

To reproduce:

Run import createEmotionServer from '@emotion/server/create-instance' in a worker or a browser environment.

It cannot be reproduced in Codesandbox because looks like they add some Node API polyfills (Buffer, etc).

Expected behavior:

It should not crash on import. More specifically, it would be great if there was an ESM build or a way to import only the necessary functions. For example, importing createExtractCriticalToChunks and createConstructStyleTagsFromChunks, and leaving out createRenderStylesToStream, which is the one causing problems and not needed for this use case.

Right now I have this code, which works in Node but not in workers.

@Andarist
Copy link
Member

With #2819 we are allowing ourselves to add a special entrypoint for workers. So we are able to fix this if somebody would be willing to provide a fix for this. That being said I think that @nicksrandall has already been testing this branch with Cloudflare Workers and he said that it's all working. So perhaps there isn't anything to be done here once this lands. On the other hand, this PR doesn't really remove dependency on the packages mentioned in this issue so maybe there is still something to be worked on here.

@aaronadamsCA
Copy link

It's still not possible to use @emotion/server/create-instance in a worker environment.

@nicksrandall, I see #2819 did not add a worker condition to the server package. Would that be a big change, or is there a reason this was left out?

In our case, it's the only apparent blocker to using Emotion + SSR in a worker environment.

@Andarist
Copy link
Member

@aaronadamsCA it might be an oversight - could you tell me how does it crash right now in the worker env? There is a chance that we depend on node-specific stuff there. In such a case, we'd have to provide an alternative implementation for workers

@aaronadamsCA
Copy link

@Andarist You guessed right! html-tokenize is heavily dependent on Node.js. I've tried polyfilling its various needs to get it to run in a worker environment, without success; it gets upset with the absence of Buffer, and doesn't seem to tolerate a polyfill.

html-tokenize appears to be a dead project; the last major release was in 2016 (with one dependency update in 2020). I think the only way forward here would be to get rid of that dependency; the discussion in #2781 seems to be on the right track.

@aaronadamsCA
Copy link

Actually - I wonder if the scope of this issue could be to fix Emotion Server in worker environments, and then the scope of #2781 (blocked by this issue) could be to add any specific worker environment capabilities.

My understanding is that @emotion/server is essentially a progressive enhancement; Emotion works fine without it by injecting styles into the client-side <head>, but if you want styles in your SSR pages during the initial load, then you need to get those tags from @emotion/server and inject them into the first render yourself.

What seems odd, then, is that @emotion/server has unique external dependencies to make this happen. Obviously Emotion already knows how to generate style tags; so why isn't @emotion/server just getting those tags from other parts of Emotion?

I think this would be well worth fixing soon, as I haven't found a viable workaround. Right now we get a massive FOUC (flash of unstyled content) on landing/reload; we can solve that with a loading indicator, but a big part of the value of SSR workers is a correct first render, and I'd really like to get that working.

@aaronadamsCA

This comment was marked as outdated.

@Andarist
Copy link
Member

The current implementation shouldn't be reused in workers as-is as conceptually it returns a node stream:

renderStylesToNodeStream(): NodeJS.ReadWriteStream

Even the method's name mention this - it's renderStylesToNodeStream after all.

I'm all for providing an alternative implementation of this that would work with web streams. For the time being, we could just have an additional renderStylesToWebStream that wouldn't depend on this html-tokenize (and other node-specific deps) at all.

Note that this whole thing is mostly needed only if you are using @emotion/css (and not @emotion/react/@emotion/styled). With React-specific APIs we could have something that wouldn't rely on any kind of extraction from the stream. We could just grab "inserted" things more directly from the per-request cache.

@aaronadamsCA

This comment was marked as outdated.

@Andarist
Copy link
Member

Even though we only use constructStyleTagsFromChunks and extractCriticalToChunks, we are still affected by the Node.js-only import of html-tokenize. That problem will need to be solved before any other improvements can be made.

Ah, ye - I can see that. We need to restructure our package a little bit or provide different bundles for specific environments. This might require some changes in our bundling solution (https://github.com/preconstruct/preconstruct/) to make it all work.

Yep, this is what I meant by "why isn't @emotion/server just getting those tags from other parts of Emotion". We'd love this.

It's on the "roadmap". However, lately, I don't get as much free time for OSS as I used to have and I'm not certain when I will be able to actually work on this.

@Andarist
Copy link
Member

Maybe the simplest solution would be to convert these imports to dynamic import() expressions inside createRenderStylesToNodeStream():

We probably can't do that because this would taint the whole function - it would have to become async and that would be a breaking change.

@aaronadamsCA
Copy link

Good point. require()?

@Andarist
Copy link
Member

I'd prefer not to mix ESM with require - it's a can of worms that I prefer closed ;p

@aaronadamsCA
Copy link

aaronadamsCA commented Jan 5, 2023

Our eventual solution was to replace @emotion/server with a "vendored" package in our monorepo.

package.json
{
  "name": "emotion-server",
  "sideEffects": false
}
index.d.ts
import type { EmotionCache } from "@emotion/utils";

interface EmotionCriticalToChunks {
  html: string;
  styles: { key: string; ids: string[]; css: string }[];
}

interface EmotionServer {
  constructStyleTagsFromChunks: (
    criticalData: EmotionCriticalToChunks
  ) => string;
  extractCriticalToChunks: (html: string) => EmotionCriticalToChunks;
}

export function createEmotionServer(cache: EmotionCache): EmotionServer;
index.js
function createExtractCriticalToChunks(cache) {
  return function (html) {
    const RGX = new RegExp(`${cache.key}-([a-zA-Z0-9-_]+)`, "gm");

    const o = { html, styles: [] };
    let match;
    const ids = {};
    while ((match = RGX.exec(html)) !== null) {
      if (ids[match[1]] === undefined) {
        ids[match[1]] = true;
      }
    }

    const regularCssIds = [];
    let regularCss = "";

    Object.keys(cache.inserted).forEach((id) => {
      if (
        (ids[id] !== undefined ||
          cache.registered[`${cache.key}-${id}`] === undefined) &&
        cache.inserted[id] !== true
      ) {
        if (cache.registered[`${cache.key}-${id}`]) {
          regularCssIds.push(id);
          regularCss += cache.inserted[id];
        } else {
          o.styles.push({
            key: `${cache.key}-global`,
            ids: [id],
            css: cache.inserted[id],
          });
        }
      }
    });

    o.styles.push({ key: cache.key, ids: regularCssIds, css: regularCss });

    return o;
  };
}

function generateStyleTag(cssKey, ids, styles, nonceString) {
  return `<style data-emotion="${cssKey} ${ids}"${nonceString}>${styles}</style>`;
}

function createConstructStyleTagsFromChunks(cache, nonceString) {
  return function (criticalData) {
    let styleTagsString = "";

    criticalData.styles.forEach((item) => {
      styleTagsString += generateStyleTag(
        item.key,
        item.ids.join(" "),
        item.css,
        nonceString
      );
    });

    return styleTagsString;
  };
}

export function createEmotionServer(cache) {
  if (cache.compat !== true) {
    cache.compat = true;
  }
  const nonceString =
    cache.nonce !== undefined ? ` nonce="${cache.nonce}"` : "";
  return {
    extractCriticalToChunks: createExtractCriticalToChunks(cache),
    constructStyleTagsFromChunks: createConstructStyleTagsFromChunks(
      cache,
      nonceString
    ),
  };
}

This exports a createEmotionServer that is a drop-in replacement for @emotion/server/create-instance in a web worker environment. It's functionally identical to the actual package, just without the Node.js-only parts we don't use.

@Nipsuli
Copy link

Nipsuli commented Jan 6, 2023

@aaronadamsCA thanks! This works like charm!

@maheshsundaram
Copy link

@aaronadamsCA Thanks! I'm using Chakra-UI in a Remix project (specifically a Hydrogen Shopify app) and had the same issue. The workaround is very helpful.

@MHarris021
Copy link

@aaronadamsCA great job. Only thing I could get to work after messing around for hours w/ Miniflare (CloudFlare local dev tool), Chakra-UI, and Remix configurations and examples. This has fixed it for local dev at least. Will let you know if it has problems with deployng.

@MHarris021
Copy link

For those using Typescript, setting up the paths object to look like below allows for additional dropins for situations like this:

paths": { "~/*": ["./app/*"], "@lib/*": ["./lib/*"] },

Anything in the root level lib directory can be imported this way. Just make sure to include an index w/ an export.

@jrschumacher
Copy link

For others who might need a little extra guidance and building on the work of @aaronadamsCA

  1. Create new directory /app/vendor/@emotion/server

  2. Add index.ts in this directory

/app/vendor/@emotion/server/index.ts
import type { EmotionCache } from "@emotion/utils";

function createExtractCriticalToChunks(cache: EmotionCache) {
  return function (html: string) {
    const RGX = new RegExp(`${cache.key}-([a-zA-Z0-9-_]+)`, "gm");

    const o: {
      html: string;
      styles: { key: string; ids: string[]; css: string | boolean }[];
    } = { html, styles: [] };
    let match;
    const ids: { [key: string]: boolean} = {};
    while ((match = RGX.exec(html)) !== null) {
      if (ids[match[1]] === undefined) {
        ids[match[1]] = true;
      }
    }

    const regularCssIds: string[] = [];
    let regularCss = "";

    Object.keys(cache.inserted).forEach((id) => {
      if (
        (ids[id] !== undefined ||
          cache.registered[`${cache.key}-${id}`] === undefined) &&
        cache.inserted[id] !== true
      ) {
        if (cache.registered[`${cache.key}-${id}`]) {
          regularCssIds.push(id);
          regularCss += cache.inserted[id];
        } else {
          o.styles.push({
            key: `${cache.key}-global`,
            ids: [id],
            css: cache.inserted[id],
          });
        }
      }
    });

    o.styles.push({ key: cache.key, ids: regularCssIds, css: regularCss });

    return o;
  };
}

function generateStyleTag(cssKey: string, ids: string, styles: string | boolean, nonceString: string) {
  return `<style data-emotion="${cssKey} ${ids}"${nonceString}>${styles}</style>`;
}

function createConstructStyleTagsFromChunks(cache: EmotionCache, nonceString: string) {
  return function (criticalData: ReturnType<ReturnType<typeof createExtractCriticalToChunks>>) {
    let styleTagsString = "";

    criticalData.styles.forEach((item) => {
      styleTagsString += generateStyleTag(
        item.key,
        item.ids.join(" "),
        item.css,
        nonceString
      );
    });

    return styleTagsString;
  };
}

export function createEmotionServer(cache: EmotionCache) {
  if (cache.compat !== true) {
    cache.compat = true;
  }
  const nonceString =
    cache.nonce !== undefined ? ` nonce="${cache.nonce}"` : "";
  return {
    extractCriticalToChunks: createExtractCriticalToChunks(cache),
    constructStyleTagsFromChunks: createConstructStyleTagsFromChunks(
      cache,
      nonceString
    ),
  };
}
  1. Update /app/entry.server.tsx to use the new Emotion server
/app.entry.server.tsx
//... existing imports

import createEmotionCache from '@emotion/cache';
import { createEmotionServer } from '~/vendor/@emotion/server';

export default function handleRequest(
) {
  const cache = createEmotionCache({ key: 'css' });
  const { extractCriticalToChunks } = createEmotionServer(cache);

  //... if using MUI boilerplate you might have MuiRemixServer function
  const html = ReactDOMServer.renderToString(/* existing component */);

  const { styles } = extractCriticalToChunks(html);

  //... injecting styles
  //... returns
}

@lorenzo-del-rosario
Copy link

So from my understanding currently the only workaround is by serving the content as a string with renderToString and add the emotion styles in the string.
There are no workarounds if we need to use renderToReadableStream, correct?

@paul-vd
Copy link

paul-vd commented Sep 25, 2024

So from my understanding currently the only workaround is by serving the content as a string with renderToString and add the emotion styles in the string. There are no workarounds if we need to use renderToReadableStream, correct?

I'm wondering the same thing, but I guess not, since the stream won't be complete unless yoyu use the stream.allReady which defies the purpose.

I'm going to play around with following two returns from the stream to see if there is anything that can be done to pipe it

const stream = await renderToReadableStream(..)
stream.pipeTo
stream.pipeThrough

This is to see if we can achieve something simular to the pipe() used here

@Andarist
Copy link
Member

I'm open to reviewing a PR implementing a fix for this. I don't have enough context to work on the fix myself at the moment though.

@lorenzo-del-rosario
Copy link

So from my understanding currently the only workaround is by serving the content as a string with renderToString and add the emotion styles in the string. There are no workarounds if we need to use renderToReadableStream, correct?

I'm wondering the same thing, but I guess not, since the stream won't be complete unless yoyu use the stream.allReady which defies the purpose.

I'm going to play around with following two returns from the stream to see if there is anything that can be done to pipe it

const stream = await renderToReadableStream(..)
stream.pipeTo
stream.pipeThrough

This is to see if we can achieve something simular to the pipe() used here

I see that the solution in the example you mention has access to the stream node module (imports PassThrough from stream and uses @emotion/server which relies on the stream package), while on worker environment is not always available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants