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

[Bug]: [REACT - Next.js] floating-ui is causing server error on SSR #2970

Open
2 of 3 tasks
maciej-jezierski opened this issue Sep 5, 2023 · 9 comments
Open
2 of 3 tasks
Labels
bug Something isn't working

Comments

@maciej-jezierski
Copy link

maciej-jezierski commented Sep 5, 2023

Describe the Bug

We set up a project with the instruction of the https://docs.storefrontui.io/v2/react/getting-started.html
After set up we added SfButton and got an intermediate error

Expected behavior

Adding any component shouldn't cause errors on SSR

Steps to reproduce

Remove "use client"; line and it starts break

app/page.tsx

import { SfButton } from "@storefront-ui/react";
import Image from "next/image";
​
export default function ButtonBlock() {
  return (
    <div>
      <Image
        src="https://www.winparts.eu/assets/img/logo.svg"
        alt="logo"
        width={208}
        height={87}
      />
      <SfButton className="!bg-sale">Hello</SfButton>
    </div>
  );
}```

### SFUI version

@storefront-ui/[email protected]

### Framework

React, Next.js

### Node version

16

### Browser

Chrome

### OS

Linux

### Relevant log output

```shell
./app/page.tsx
ReactServerComponentsError:

You're importing a component that needs useEffect. It only works in a Client Component but none of its parents are marked with "use client", so they're Server Components by default.
Learn more: https://nextjs.org/docs/getting-started/react-essentials

   ,-[/home/bravebison/winparts-storefront/node_modules/@floating-ui/react-dom/dist/floating-ui.react-dom.esm.js:1:1]
 1 | import { arrow as arrow$1, computePosition } from '@floating-ui/dom';
 2 | export { autoPlacement, autoUpdate, computePosition, detectOverflow, flip, getOverflowAncestors, hide, inline, limitShift, offset, platform, shift, size } from '@floating-ui/dom';
 3 | import * as React from 'react';
 4 | import { useLayoutEffect, useEffect } from 'react';
   :                           ^^^^^^^^^
 5 | import * as ReactDOM from 'react-dom';
 6 | 
 7 | /**
   `----

The error was caused by importing '@storefront-ui/react/dist/index.mjs' in './app/page.tsx'.

Maybe one of these should be marked as a client entry with "use client":
  ./app/page.tsx

Able to fix / change the documentation?

  • Yes
  • No

Code of Conduct

  • I agree to follow this project's Code of Conduct
@maciej-jezierski maciej-jezierski added the bug Something isn't working label Sep 5, 2023
@Szymon-dziewonski
Copy link
Contributor

Hello @maciej-jezierski , thank you for creating issue ticket, as far as I know since next 13, you have to differentiate server side and client side component with use client attribute, by default all components are server side which means floating-ui won't work. For me its desired outcome. However since I'm not react expect @dkacper could you please lend your expertise here please ? Thank

@maciej-jezierski
Copy link
Author

hey @Szymon-dziewonski the problem in the above code we didn't use floating ui, yet it was invoked. This is the only component as we are testing waters now.

@karimshalapy
Copy link

I know that in next we need to explicitly mention which components are client component because the interactivity can not happen on the server.
I am not experienced with Storefront UI, but from what I see, the SFButton component takes in an as prop, and in this prop I can pass the NextLink and if this is a valid usage, then shouldn't this element renderable on the server?

I will copy the source code of the button and try some stuff then report the findings here.

@karimshalapy
Copy link

In the current project, we copied the source code of the button and made our own custom button from the storefront base.
After a bit of investigation, I found out that the import statement from storefront UI is causing this error to happen, although all we import is types and enums.

import {
  type SfButtonProps as OriginalSfButtonProps,
  SfButtonSize,
  SfButtonVariant,
  polymorphicForwardRef
} from '@storefront-ui/react';

After trying out some stuff, it became apparent that importing and using anything from @storefront-ui/react that's not prefixed with the type keyword in the import will, in fact, use useEffect and throw the error mentioned above.

In other words, I can simply use the normal forwardRef, and define the SfButtonSize and SfButtonVariant myself and I wouldn't have this issue.

@karimshalapy
Copy link

And I think if the Storefront UI components should only be rendered on the client, wouldn't it be better to add "use client" in the exported components?
Material UI does this as you can see here: https://github.com/mui/material-ui/blob/master/packages/mui-base/src/Button/Button.tsx

@dkacper
Copy link
Contributor

dkacper commented Sep 11, 2023

We've been investigating this issue a bit and went to the same conclusion as @karimshalapy. Using any component from the SFUI simply does not work as RSC at the moment. Unfortunately, We can't suggest anything better than using use client directive for now. We're looking forward to hear suggestions on how to deal with it. Meanwhile, we'll be looking into this as well.

@Szymon-dziewonski
Copy link
Contributor

And I think if the Storefront UI components should only be rendered on the client, wouldn't it be better to add "use client" in the exported components? Material UI does this as you can see here: https://github.com/mui/material-ui/blob/master/packages/mui-base/src/Button/Button.tsx

@karimshalapy use client is next 13 only specific thing, component can be run in pure react, astro, gatsby etc. So we would like to not constrain ourselves to only one meta-framework. However there seems to be other issue here like @dkacper replied.

@maciej-jezierski
Copy link
Author

@Szymon-dziewonski If all components are available only on the client side it vastly limits the usability of the framework on production usage in ecommerce where clients place very high importance on SEO and links should be ready once server rendering is finished.

@Szymon-dziewonski
Copy link
Contributor

@maciej-jezierski I understand that, totally agree with you, some of components like button or chip etc should be used sever side. On the other hand there are components like tooltip that can't be run server side, just because there is no browser context need to run specific operations.

At the end, we are not yet know what is the issue but for sure we will investigate. I do appreciate your contribution here, we definitely will check this out, once we will have some answers we will respond here. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants