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

Supporting Emotion and The Next.js Edge Runtime #39229

Closed
1 task done
ItsWendell opened this issue Aug 1, 2022 · 22 comments
Closed
1 task done

Supporting Emotion and The Next.js Edge Runtime #39229

ItsWendell opened this issue Aug 1, 2022 · 22 comments
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@ItsWendell
Copy link

ItsWendell commented Aug 1, 2022

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP PREEMPT_DYNAMIC Fri Jul 22 14:03:36 UTC 2022
Binaries:
  Node: 16.15.0
  npm: 8.5.5
  Yarn: N/A
  pnpm: 7.1.7
Relevant packages:
  next: 12.2.4-canary.8
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

Emotion recently implemented workers support, which means that it should work for the Edge Runtime of Next.js. I've quickly tested this with the latest version of Emotion (emotion-js/emotion#2819), since the worker runtime should have the same APIs as the Edge Runtime of Verce / Next.js.

Here's a more advanced repository to test Emotion / Chakra with Edge Runtime support:
https://github.com/ItsWendell/next-edge-runtime-chakra-ui-emotion

While a specific worker bundle output is available for emotion now, the edge runtime/compiler doesn't seem to use it.

ReferenceError: document is not defined

This error happened while generating the page. Any console logs will be displayed in the terminal window.
Call Stack
<unknown>
node_modules/@emotion/cache/dist/emotion-cache.browser.esm.js (199:0)
eval
node_modules/@emotion/cache/dist/emotion-cache.browser.esm.js (199:0)

Emotion exports separate bundles for workers, e.g. *.worker.esm.js, which might also work with the Edge Runtime.

I would love to see what it requires to make Emotion plug and play for the Edge Runtime of Next.js / Vercel, this will open up the door for a lot of UI libraries like Chakra-UI to be compatible with it rendering on the edge too!

Expected Behavior

Next.js uses the worker bundle with the edge-runtime enabled.

Link to reproduction

https://github.com/ItsWendell/next-emotion-cache-edge-runtime

To Reproduce

  • Start up the reproduction repository
  • The page will output the following server error:
Server Error
ReferenceError: document is not defined

This error happened while generating the page. Any console logs will be displayed in the terminal window.
Call Stack
<unknown>
node_modules/@emotion/cache/dist/emotion-cache.browser.esm.js (199:0)
eval
node_modules/@emotion/cache/dist/emotion-cache.browser.esm.js (199:0)

I've opened a similar comment at emotion-js/emotion#2801

@Andarist
Copy link
Contributor

Andarist commented Aug 2, 2022

Emotion maintainer here 👋

Note that we forgot to add a worker condition in @emotion/cache. So it's not that surprising that this particular package still didn't work here.

I've just released a new version of this package, with the appropriate condition, could you check if this has fixed your issue?

@balazsorban44
Copy link
Member

Thanks, @Andarist for the update! Currently, we are picking the browser field to target the runtime, but there is a discussion on our side to maybe favor a special condition that describes our runtime better. 👍

@balazsorban44 balazsorban44 added Runtime Related to Node.js or Edge Runtime with Next.js. area: Ecosystem labels Aug 2, 2022
@ItsWendell
Copy link
Author

@balazsorban44 I think a condition that favors Web-interoperable Runtimes (WinterCG) might even be better, since Vercel, Cloudflare and Deno are also a part of that.

@huozhi
Copy link
Member

huozhi commented Aug 2, 2022

Edge runtime doesn't support DOM api like document, and the browser bundle of @emotion/cache removes those checks like typeof document !== undefined that assuming browser bundle will always have DOM API.

@Andarist I wonder if this behavior could changed, such as not tree shaking those conditions, and let runtime to handle it. I tested if I add those checks back, then it won't crash and @emotion/styled can work on client. I guess there still some changes have to be done for SSR.

Another idea is to spearate the exports into different subpath imports, like what react-dom does for react-dom/server and react-dom/client and make it more specific for runtime. And only /client can be used in browser.

@Andarist
Copy link
Contributor

Andarist commented Aug 2, 2022

@Andarist I wonder if this behavior could changed, such as not tree shaking those conditions, and let runtime to handle it. I tested if I add those checks back, then it won't crash and @emotion/styled can work on client. I guess there still some changes have to be done for SSR.

Technically we could remove the browser-specific bundles but then we'd regress a little bit on the bundlesize. I would prefer not to do that.

Another idea is to spearate the exports into different subpath imports, like what react-dom does for react-dom/server and react-dom/client and make it more specific for runtime. And only /client can be used in browser.

I don't think this is a feasible solution for us (note that for some server-only stuff we have a completely separate package already). The idea behind the structure of our packages is that their API surface is the same, regardless of the target environment. Our packages are also often wrapped by other packages (3rd party ones) and "repackaged" - for instance, the user of Chakra UI might not even interact with Emotion directly, even though Chakra UI is built on top of Emotion. Given that - all of our consumers would have to restructure their code to accommodate for such a change. It's much easier for us and our consumers to have the same entrypoint for all targets and for the per-target code to be picked by a bundler or at runtime.

Thanks, @Andarist for the update! Currently, we are picking the browser field to target the runtime, but there is a discussion on our side to maybe favor a special condition that describes our runtime better. 👍

I think that it makes sense for you to support a worker condition, as mentioned here. I don't quite see what could go wrong here - there are certainly not a lot of packages that are using it and if a package already uses this condition I don't imagine a situation in which choosing their browser bundler over a worker bundle would be better in this context.

I understand that, in the long run, worker condition might not be "enough". Maybe will have to eventually differentiate between webworkers and edge workers - I don't think it's the case right now though. On top of that, this acts as a priority list, sort of, so by adding support for the worker condition in Next we are not limiting our future selves. Introducing a new condition will be easy and won't require any breaking changes.

Note that I don't think that you should immediately remove the support for the browser condition in this context - maybe it should stay here forever, but supporting both at the same time allows package maintainers to add more-specific bundles for workers than the ones already prepared for browsers.

@huozhi
Copy link
Member

huozhi commented Aug 2, 2022

Removing the browser field also sounds a way to go, I think bundler can still optimize those conditions when emits client bundle, and do proper tree shake to keep the size aligned with "browser" bundle.

@Andarist
Copy link
Contributor

Andarist commented Aug 2, 2022

If you suggest that Emotion should remove a browser field from its conditions then I don't think it's a viable strategy for us. It shifts the responsibility to drop the unused code to the bundler. The problem with that is that not every bundler does this out of the box. It would also limit us from introducing more complex code differences between those targets - mainly because we'd be limited to conditional statements and stuff but sometimes it's nice to swap out whole files or even dependencies.

@ItsWendell
Copy link
Author

@Andarist would it be worth seeing/testing what the bundle size/performance impact would be to keep these checks in place for e.g. document / window availability within the browser bundle?

Other than that the core API of the browser/web environments, whether it's browser, workers, deno should be intentionlly the same.

@Andarist
Copy link
Contributor

Andarist commented Aug 2, 2022

@Andarist would it be worth seeing/testing what the bundle size/performance impact would be to keep these checks in place for e.g. document / window availability within the browser bundle?

I don't think this is worth testing - it's also always subject to change. I don't see a compelling reason to degrade any aspect of the implementation to improve the compatibility with other tools when there is a somewhat clear path towards a better way of doing those things. exports have been purposefully designed to enable such things and I feel like this is the right answer here - the consuming tools, like bundlers, just have to pick up the right condition to utilize here (although I was under the impression that worker conditions was already used by some). This will benefit more libraries and more tools in the long run, setting up a de facto standard. While we can bikeshed how Emotion could deal with this it won't solve the problem at scale.

Other than that the core API of the browser/web environments, whether it's browser, workers, deno should be intentionlly the same.

And the public API of Emotion is the same, regardless of the environment.

@ItsWendell
Copy link
Author

Here's an example of a custom webpack configuration / next.config.js that actually made this work in some sense, we add resolve conditionName worker and some defaults if they don't exist yet.

module.exports = async (phase, { defaultConfig }) => {
    /**
     * @type {import('next').NextConfig}
     */
    const nextConfig = {
        reactStrictMode: true,
        compiler: {
            emotion: true,
        },
        webpack: (config, ctx) => {
            if (ctx.nextRuntime === "edge") {
                if (!config.resolve.conditionNames) {
                    config.resolve.conditionNames = ['require', 'node'];
                }
                if (!config.resolve.conditionNames.includes("worker")) {
                    config.resolve.conditionNames.push("worker");
                }
            }
            return config;
        },
        experimental: {
            runtime: 'experimental-edge',
        },
    }
    return nextConfig
}

@ItsWendell
Copy link
Author

Thanks, @Andarist for the update! Currently, we are picking the browser field to target the runtime, but there is a discussion on our side to maybe favor a special condition that describes our runtime better. +1

Is there a separate issue tracking this or is this an internal conversation? Curious what will be decided on!

@mwskwong
Copy link

Just want to point out that it used to work on an older version of Next.js, but after a certain minor version (forgot about the exact version), Emotion stops working with edge runtime again.

@mwskwong
Copy link

mwskwong commented Dec 6, 2022

Here's an example of a custom webpack configuration / next.config.js that actually made this work in some sense, we add resolve conditionName worker and some defaults if they don't exist yet.

module.exports = async (phase, { defaultConfig }) => {
    /**
     * @type {import('next').NextConfig}
     */
    const nextConfig = {
        reactStrictMode: true,
        compiler: {
            emotion: true,
        },
        webpack: (config, ctx) => {
            if (ctx.nextRuntime === "edge") {
                if (!config.resolve.conditionNames) {
                    config.resolve.conditionNames = ['require', 'node'];
                }
                if (!config.resolve.conditionNames.includes("worker")) {
                    config.resolve.conditionNames.push("worker");
                }
            }
            return config;
        },
        experimental: {
            runtime: 'experimental-edge',
        },
    }
    return nextConfig
}

This doesn't seem to be working anymore

@Gisleburt
Copy link

Just a heads up that the above solution does work on Next 12 BUT there's no window and URL is overwritten by Next (I think) so if your library depends on either of those (eg, both Launch Darkly's browser and server libraries) then it still won't work in middleware, you might need to find somewhere else to put the code.

@capJavert
Copy link

capJavert commented Jan 18, 2023

Here's an example of a custom webpack configuration / next.config.js that actually made this work in some sense, we add resolve conditionName worker and some defaults if they don't exist yet.

module.exports = async (phase, { defaultConfig }) => {
    /**
     * @type {import('next').NextConfig}
     */
    const nextConfig = {
        reactStrictMode: true,
        compiler: {
            emotion: true,
        },
        webpack: (config, ctx) => {
            if (ctx.nextRuntime === "edge") {
                if (!config.resolve.conditionNames) {
                    config.resolve.conditionNames = ['require', 'node'];
                }
                if (!config.resolve.conditionNames.includes("worker")) {
                    config.resolve.conditionNames.push("worker");
                }
            }
            return config;
        },
        experimental: {
            runtime: 'experimental-edge',
        },
    }
    return nextConfig
}

This works ✅ for me on 13.1.2 on the page with material-ui (joy), emotion and getStaticProps (ISR).

EDIT: It is also worth to say that next build prints out "getStaticProps" is not yet supported fully with "experimental-edge"

@Andarist
Copy link
Contributor

@huozhi would you consider enabling 'worker' condition by default in this edge environment? This is suggested by the webpack docs here. While workers are not exactly webworkers - the web part is put in parenthesis and edge runtimes are aiming for compat with the web platform anyway. So I think that this is a reasonable condition for those environments.

@github-actions github-actions bot added the linear: next Confirmed issue that is tracked by the Next.js team. label Jan 19, 2023
@huozhi
Copy link
Member

huozhi commented Feb 6, 2023

Looks like we have a standard from https://runtime-keys.proposal.wintercg.org/ which recommends "edge-light" as the field. I think edge runtime is still different from web workers, so would be great if we can have a edge-light field for emotion 🙏

@Andarist
Copy link
Contributor

Andarist commented Feb 6, 2023

We could do that - but arguably... browser is also different from edge-light and, IIRC, you resolve browser when bundling for workers.

I agree that edge-light might be a good addition when somebody wants to provide different files for edge-light and worker but when that is not needed... worker could still be selected for edge workers.

@huozhi
Copy link
Member

huozhi commented Feb 6, 2023

Will reach out to team to see if we can pick up the "edge-light" for edge runtime

@huozhi
Copy link
Member

huozhi commented Feb 16, 2023

picking edgt-light field for package in #45188

@Andarist
Copy link
Contributor

I believe that this can now be closed. Emotion provides a worker condition and that has been enabled for the Edge runtime in this PR: #46066

@huozhi huozhi closed this as completed Feb 19, 2023
@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
Development

No branches or pull requests

8 participants