fluentui icon indicating copy to clipboard operation
fluentui copied to clipboard

[Bug]: v9's Provider doesn’t hydrate from SSR properly in StrictMode

Open thure opened this issue 2 years ago • 10 comments

Library

React Components / v9 (@fluentui/react-components)

System Info

System:
    OS: macOS 12.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 4.38 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Browsers:
    Chrome: 102.0.5005.115
    Firefox: 101.0.1
    Safari: 15.5

My project uses PNPM so envinfo might have missed: @fluentui/react-components v9.0.0-rc.14

Are you reporting Accessibility issue?

no

Reproduction

https://github.com/OfficeDev/fluent-blocks/tree/feat/sample-app

Bug Description

Actual Behavior

The Provider won’t apply styles in certain situations when it’s been rehydrated. Sometimes when hot reloading certain styles aren’t applied, and certain styles are always missing from tooltips and menus which v9 renders in a sibling provider with a different-numbered class name from the primary provider (e.g. the primary will have fui-FluentProvider1, but tooltip & menu providers will have fui-FluentProvider2).

pr

This could possibly be construed as an accessibility bug since it causes tooltips & menus to blur and unmount right after mounting.

Expected Behavior

In Storybook and other SPA situations the Provider behaves as expected – it seems aware that other providers have been rendered, and when new tooltips or menus appear they appear in a provider element with the same n in their class name as their relevant provider sibling, e.g. 57 in fui-FluentProvider57.

Steps to reproduce

I'm working on getting a truly minimal reproduction running on Vercel (this can’t be reproduced on CodeSandbox since it’s an SSR issue) but wanted to log this in case I missed something and this is easily resolved.

Logs

> @fluent-blocks/[email protected] dev /Users/willshown/Projects/fluent-blocks/packages/react-sample-app
> next dev

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
wait  - compiling...
event - compiled client and server successfully in 5.7s (2167 modules)
wait  - compiling / (client and server)...
wait  - compiling...
event - compiled client and server successfully in 1479 ms (2174 modules)

Requested priority

High

Products/sites affected

Fluent Blocks, Teams Developer Portal

Are you willing to submit a PR to fix?

yes

Validations

  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

thure avatar Jun 21 '22 07:06 thure

Did you follow the setup guide https://react.fluentui.dev/?path=/docs/concepts-developer-server-side-rendering--page?

I don't see SSRProvider used anywhere near to RendererProvider...

layershifter avatar Jun 21 '22 15:06 layershifter

I'm working on getting a truly minimal reproduction running on Vercel (this can’t be reproduced on CodeSandbox since it’s an SSR issue) but wanted to log this in case I missed something and this is easily resolved.

FYI: You can create containers with Next.js on CodeSandbox:

image

layershifter avatar Jun 21 '22 15:06 layershifter

Your point about SSRProvider is well taken since I'd instead been following Griffel’s docs on SSR, though after adding v9’s SSRProvider (and clearing Next.js’s caches, etc):

  • there are no issues in production builds, but development mode still has the same issue…
    • I have attempted a reproduction on CodeSandbox, but I haven’t been able to reproduce the issue there…
    • I think this bug can be closed for now since at worst only the dev environment is affected; I'll continue trying to reproduce the issue in CodeSandbox as long as it persists for me
  • I now notice the message missing about using SSRProvider; I didn’t guess that message was coming from v9 – I imagine it'd help developers in a similar situation if the message declared it was coming from @fluentui/react-components and linked to the documentation you helpfully provided in this thread
  • ⚠️ The types for RendererProvider and SSRProvider don’t permit children, I had to find their props and cast them to FC<PropsWithChildren<…>> to use them as prescribed.

thure avatar Jun 21 '22 22:06 thure

there are no issues in production builds, but development mode still has the same issue…

I see that you are using React 18 (at least in CodeSandbox). There is a problem with StrictMode, #23625. Can you please check if you are using it? (it would explain why only development builds are affected).

I now notice the message missing about using SSRProvider; I didn’t guess that message was coming from v9 – I imagine it'd help developers in a similar situation if the message declared it was coming from @fluentui/react-components and linked to the documentation you helpfully provided in this thread

Makes sense, I will create a PR to modify it.

  • ⚠️ The types for RendererProvider and SSRProvider don’t permit children, I had to find their props and cast them to FC<PropsWithChildren<…>> to use them as prescribed.

I think that it's because you use @types/react@18, React.FC there does not have children by default there anymore. Good catch. I modified https://github.com/microsoft/griffel/issues/40 to include that.

layershifter avatar Jun 22 '22 08:06 layershifter

I used SSRProvider but I still got this problem. In production build everything runs perfectly but in dev there's no CSS variable applied. I thought it is a HMR problem but in CodeSandBox, it runs fine. Weird.

AkiraVoid avatar Aug 08 '22 10:08 AkiraVoid

@AkiraVoid the workaround is to disable Strict mode:

// next.config.js
// https://nextjs.org/docs/api-reference/next.config.js/react-strict-mode
module.exports = {
  reactStrictMode: false,
}

~~However it should be fixed in @fluentui/[email protected] by #24061. Can you please check?~~

Edit: not really, it's a different problem. You can disable StrictMode and it solves the problem, but we need to make it working properly even within it...

layershifter avatar Aug 08 '22 10:08 layershifter

@layershifter I am actually using 9.2.0, so I don’t think it has been fixed.

AkiraVoid avatar Aug 08 '22 15:08 AkiraVoid

@AkiraVoid ^ check the recent edit 🐱 I just checked and indeed the only solution for now is to disable StrictMode.

layershifter avatar Aug 08 '22 15:08 layershifter

And, since v8, the numbers after class names seem to be a problem in SSR (also mentioned in this thread). Should I open another issue about this?

AkiraVoid avatar Aug 08 '22 15:08 AkiraVoid

And, since v8, the numbers after class names seem to be a problem in SSR (also mentioned in this thread). Should I open another issue about this?

Nope, we will track this problem there. To be honest, v8 has a bit different issue than we have there. For context, it's more or less the same as in other libraries (https://github.com/chakra-ui/chakra-ui/issues/4328#issuecomment-920884182).

For now, I will put a warning into my existing PR to docs to highlight the problem.

layershifter avatar Aug 08 '22 15:08 layershifter

@layershifter i tried the example on https://react.fluentui.dev/?path=/docs/concepts-developer-server-side-rendering--page for Nextjs + Typescript. I get an error in _app.tsx "Property renderer does not exist on type 'AppProps<{}>'". Do you have any suggestion for a solution? Would be nice if you could add some TS examples to the docs page.

This is how my code looks like

import React from 'react'
import { createDOMRenderer, RendererProvider, SSRProvider } from '@fluentui/react-components'
import type { AppProps } from 'next/app'

const MyApp = ({ Component, pageProps, renderer }: AppProps): JSX.Element => {
  return (
      <RendererProvider renderer={renderer || createDOMRenderer()}>
          <SSRProvider>
             <Component {...pageProps} />
          </SSRProvider>
      </RendererProvider>
  )
}

export default MyApp

EdgarKisman avatar Aug 19 '22 08:08 EdgarKisman

@EdgarKisman workaround:

- const MyApp = ({ Component, pageProps, renderer }: AppProps): JSX.Element => {
+ const MyApp = ({ Component, pageProps, renderer }: AppProps & { renderer?: GriffelRenderer }): JSX.Element => {

While GriffelRenderer will be exported from @fluentui/react-components since v9.3.0 (I'm not sure if the version number will be v9.3.0, anyway, see #24202 ). If it is not exported in your version, you can use any instead (although it is not recommended).

And by the way, this issue is not a place for this problem, you can open another issue or search for an existing issue about this problem, like #24158 .

AkiraVoid avatar Aug 19 '22 08:08 AkiraVoid

@layershifter i tried the example on https://react.fluentui.dev/?path=/docs/concepts-developer-server-side-rendering--page for Nextjs + Typescript. I get an error in _app.tsx "Property renderer does not exist on type 'AppProps<{}>'". Do you have any suggestion for a solution? Would be nice if you could add some TS examples to the docs page.

This is how my code looks like

import React from 'react'
import { createDOMRenderer, RendererProvider, SSRProvider } from '@fluentui/react-components'
import type { AppProps } from 'next/app'

const MyApp = ({ Component, pageProps, renderer }: AppProps): JSX.Element => {
  return (
      <RendererProvider renderer={renderer || createDOMRenderer()}>
          <SSRProvider>
             <Component {...pageProps} />
          </SSRProvider>
      </RendererProvider>
  )
}

export default MyApp

@EdgarKisman please check #24202. As @AkiraVoid pointed out 9.3.0 is not out yet, but you can temporary use any.

layershifter avatar Aug 19 '22 09:08 layershifter

-- Backlog review

@sopranopillow is doing the main work on this topic right now, so please continue focusing on this and collaborate with @layershifter on it.

tudorpopams avatar Dec 05 '22 17:12 tudorpopams