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]: v9's Provider doesn’t hydrate from SSR properly in StrictMode #23620

Closed
2 tasks done
thure opened this issue Jun 21, 2022 · 14 comments · Fixed by #26465
Closed
2 tasks done

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

thure opened this issue Jun 21, 2022 · 14 comments · Fixed by #26465

Comments

@thure
Copy link

thure commented Jun 21, 2022

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/react-sample-app@0.1.0 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

  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@layershifter
Copy link
Member

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
Copy link
Member

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

@thure
Copy link
Author

thure commented Jun 21, 2022

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.

@layershifter
Copy link
Member

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 microsoft/griffel#40 to include that.

@AkiraVoid
Copy link

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.

@layershifter
Copy link
Member

layershifter commented Aug 8, 2022

@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/react-components@9.2.0 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...

@AkiraVoid
Copy link

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

@layershifter
Copy link
Member

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

@layershifter layershifter changed the title [Bug]: v9's Provider doesn’t hydrate from SSR properly [Bug]: v9's Provider doesn’t hydrate from SSR properly in StrictMode Aug 8, 2022
@AkiraVoid
Copy link

AkiraVoid commented Aug 8, 2022

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?

@layershifter
Copy link
Member

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 (chakra-ui/chakra-ui#4328 (comment)).

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

@EdgarKisman

This comment was marked as off-topic.

@AkiraVoid

This comment was marked as off-topic.

@layershifter

This comment was marked as off-topic.

@tudorpopams
Copy link
Contributor

-- 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.

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

Successfully merging a pull request may close this issue.

9 participants