classnames icon indicating copy to clipboard operation
classnames copied to clipboard

Published 20 hours ago • verified publisher icon JedWatson

SSR Support

Open 9001-Sols opened this issue 2 years ago • 2 comments

This adds a check for window to support SSR uses.

9001-Sols avatar May 18 '22 00:05 9001-Sols

@9001-Sols why aren't you using node_modules?

dcousens avatar May 18 '22 01:05 dcousens

@9001-Sols why aren't you using node_modules?

Sorry I don't follow. Currently we import it via:

package.json:

  "dependencies": {
    "classnames": "^2.2.6",
    ...

import classNames from 'classnames';

Just the existence of window. without the typeof check causes SSR frameworks to breakdown. It's currently happening with Shopify Hydrogen (built on Vite). I haven't tested other frameworks like Next.

9001-Sols avatar May 18 '22 02:05 9001-Sols

@9001-Sols the change itself is non-controversial, but, I don't understand what you mean by

causes SSR frameworks to breakdown

The window.classNames = branch should never come to pass - if module.exports is defined - in a server environment.

dcousens avatar Sep 19 '22 22:09 dcousens

In a ES module context, module.exports does not exist.

I don't know about Hydrogen, but Vite is pure ESM if I understand correctly. So in the end this issue is more about ESM support I think.

gdostie avatar Jan 16 '23 23:01 gdostie

We do have an ESM branch next, but it's low priority typically depending on user demand

dcousens avatar Jan 17 '23 00:01 dcousens

I think the demand is definitely there, but currently it might be hard to see. When users see ESM support is missing from classnames, my guess is that people are just switching to https://www.npmjs.com/package/clsx.

gdostie avatar Jan 17 '23 14:01 gdostie

I don't know about Hydrogen, but Vite is pure ESM if I understand correctly.

Vite supports CommonJS packages, but it will transpile them to ESM using a proccess called Dependency Pre-Bundling.

I am not quite sure what this PR would accomplish, it seems to me that with this patch classNames is never exported in any manner at all, since none of the if statements are executed. I'll consider the real solution to support ESM, something that I will try to put some effort into doing in a backwards compatible manner.

Closing this for now, as I don't think this actually fixes the issue. If you can reproduce this problem reliably, please consider logging a new issue with a minimal example to reproduce it.

jonkoops avatar Dec 14 '23 16:12 jonkoops