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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TextField] Bad behavior when user edits box before hydration with SSR #34174

Open
2 tasks done
thomasjm opened this issue Sep 2, 2022 · 19 comments
Open
2 tasks done
Labels
breaking change component: text field This is the name of the generic UI component, not the React module! new feature New feature or request package: material-ui Specific to @mui/material

Comments

@thomasjm
Copy link

thomasjm commented Sep 2, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

I'm using MUI's TextField to make a simple SSR login page with a username and password.

The problem is when the user is working over a slow network connection, so it takes some time for the Javascript to download and hydrate the page. As a result, they can edit the text box before hydration occurs.

In the picture below, the user has typed "bob" for the username prior to hydration. The helper text remains in place and it looks bad:

Screenshot from 2022-09-01 19-47-09

There is also a second problem: once hydration does complete, the TextField reverts to blank, losing the user's edits, which is not good UX.

Expected behavior 馃

The TextField should behave in a reasonable way prior to hydration. Reasonable could mean either a) don't allow edits, or b) handle the edits gracefully, with good styling and without losing them upon hydration.

Steps to reproduce 馃暪

I am doing my SSR setup with Razzle.js. I made a fork here to showcase this.

  1. Clone and build the repro:
git clone git@github.com:thomasjm/razzle.git
cd razzle/examples/with-material-ui
git checkout textfield-ssr
npm install
npm run build # Press Y
cd build
node server.js
  1. Open localhost:3000 in Chrome. Open the Chrome DevTools, go to the Network tab, and set the speed to "Slow 3G".
  2. Refresh the page. Before the JS finishes downloading, try typing in the text box.

Context 馃敠

SSR is a popular technique for achieving fast page loads, and TextField should work gracefully with it.

Your environment 馃寧

npx @mui/envinfo

  System:
    OS: Linux 5.15 Ubuntu 22.04.1 LTS 22.04.1 LTS (Jammy Jellyfish)
  Binaries:
    Node: 16.15.0 - ~/.nvm/versions/node/v16.15.0/bin/node
    Yarn: 1.22.17 - ~/codedown/frontend/node_modules/.bin/yarn
    npm: 8.5.5 - ~/.nvm/versions/node/v16.15.0/bin/npm
  Browsers:
    Chrome: 104.0.5112.79
    Firefox: 104.0.1
  npmPackages:
    @emotion/react: ^11.8.2 => 11.8.2
    @emotion/styled: ^11.8.1 => 11.8.1
    @mui/base:  5.0.0-alpha.95
    @mui/core-downloads-tracker:  5.10.3
    @mui/icons-material:  5.0.5
    @mui/material: ^5.10.3 => 5.10.3
    @mui/private-theming:  5.10.3
    @mui/styled-engine:  5.10.3
    @mui/styles:  5.5.3
    @mui/system:  5.10.3
    @mui/types:  7.2.0
    @mui/utils:  5.10.3
    @types/react: ^17.0.43 => 17.0.43
    react: ^17.0.1 => 17.0.2
    react-dom: ^17.0.1 => 17.0.2
    typescript: ^4.8.2 => 4.8.2

@thomasjm thomasjm added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 2, 2022
@siriwatknp siriwatknp added new feature New feature or request component: text field This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 2, 2022
@siriwatknp
Copy link
Member

I think the solution to this is to lean toward CSS :placeholder-shown but this will be a very big breaking change for Material UI. https://codesandbox.io/s/poc-textfield-hsos3?file=/index.js

I plan to try this out for Joy UI and wait to see the feedbacks first.

@thomasjm
Copy link
Author

thomasjm commented Sep 2, 2022

Doing the placeholder with CSS sounds like it would help the styling issue. But what about the part where hydration blows away the user's edits? Would it be possible to fire an onChange event after hydration to provide the pre-hydration value?

@siriwatknp
Copy link
Member

But what about the part where hydration blows away the user's edits? Would it be possible to fire an onChange event after hydration to provide the pre-hydration value?

My gut feeling is that React should handle this properly. What's the behavior of using pure React HTML input?

@thomasjm
Copy link
Author

thomasjm commented Sep 4, 2022

Using a raw <input> doesn't work properly either actually. What happens is the pre-hydration edits stay in the box, until you trigger a React update and then they get blown away. There is no onChange event.

I'm a bit confused why nobody else seems to have problems with this issue. It seems like edits during hydration delays should affect any mutable DOM element. Do people just minimize their hydration delays and live with this?

I guess the fundamental assumption of hydration is that the pre-hydration DOM exactly matches the server-rendered DOM. The docs have the following line:

There are no guarantees that attribute differences will be patched up in case of mismatches.

@siriwatknp
Copy link
Member

Using a raw doesn't work properly either actually. What happens is the pre-hydration edits stay in the box, until you trigger a React update and then they get blown away. There is no onChange event.

Maybe @eps1lon could shed some light on this.

@thomasjm
Copy link
Author

Friendly ping @eps1lon, still hoping to make progress on this.

@eps1lon
Copy link
Member

eps1lon commented Sep 19, 2022

I think this is a problem with React inputs in general. I'm re-opening facebook/react#12955 since that's the same underlying issue.

@eps1lon
Copy link
Member

eps1lon commented Sep 19, 2022

Actually, I would recommend providing a minimal repro with just using React 18 first. React 18+ can yield during hydration. Typing into de-hydrated inputs might actually defer hydration of said input fields. Or they put heuristics into place that fixes these issues. But it warrants an investigation first.

@thomasjm
Copy link
Author

thomasjm commented Sep 23, 2022

Okay, I tried it with React 18 and got the same result I had with React 17. I now have two branches of the original repro I posted:

https://github.com/thomasjm/razzle/tree/textfield-ssr-rawinput-react17/examples/with-material-ui
https://github.com/thomasjm/razzle/tree/textfield-ssr-rawinput-react18/examples/with-material-ui

FWIW, I got the following suggested solution from StackOverflow. If the issue can't be fixed on a React-wide basis, maybe this solution could be used in TextField? https://stackoverflow.com/a/73821240/2659595 [NOTE: I just tried this in my repro scenario and it does work.]

@eps1lon
Copy link
Member

eps1lon commented Sep 24, 2022

thomasjm/razzle@textfield-ssr-rawinput-react17/examples/with-material-ui

The repro returns a 404. But I would recommend not using MUI at all to demonstrate the issue with React itself.

@thomasjm
Copy link
Author

Whoops, forgot to push the React 17 branch. Pushed now.

These repros don't use MUI, as you can see here. But you can easily comment the raw input and uncomment the TextField part to see the MUI behavior. (Well, I guess it uses <Paper> but that shouldn't be relevant.)

@eps1lon
Copy link
Member

eps1lon commented Sep 24, 2022

(Well, I guess it uses but that shouldn't be relevant.)

It's really important that these repros have as little context as possible. Try to reproduce the code to a bare minimum. There shouldn't even be a mention of MUI.

@thomasjm
Copy link
Author

thomasjm commented Sep 24, 2022

All right, repros are pretty minimal now.

EDIT: oh, I just noticed an interesting warning on React 18. Let me deal with this first...

index.js:1 Warning: ReactDOM.hydrate is no longer supported in React 18. Use hydrateRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://reactjs.org/link/switch-to-createroot

EDIT 2: okay, now the React 18 branch is using the new hydrateRoot API. The repro behavior is unchanged.

@thomasjm
Copy link
Author

thomasjm commented Oct 3, 2022

@eps1lon just checking in, is the above sufficient to reopen the React issue?

@eps1lon
Copy link
Member

eps1lon commented Oct 5, 2022

Ideally this is a Codesandbox where the tester can control when hydration happens. Remember, make it as simple as possible for somebody without your context, to understand what the issue is.

@thomasjm
Copy link
Author

thomasjm commented Oct 5, 2022

I just played with CodeSandbox a bit and had trouble finding a way to make SSR work, let alone having hydration occur on command. I'd rather not spend forever working through the nuances of CodeSandbox.

But I moved the repros into a repo of their own, with clear instructions. It's very easy to try. What do you think?

https://github.com/thomasjm/textfield-repro/tree/react-17

@iMrDJAi
Copy link

iMrDJAi commented Nov 27, 2022

Same issue here! Looks like if you start typing inside text inputs before hydration finishes, the value doesn't change till you refocus. Note that you'll still get a change event every time you press a key, and right after it, another event comes with an empty value, which indicates that something is constantly clearing the input onchange.

image

@Dragon-Huang0403
Copy link

I have a similar issue.
When I get value from local storage as a useState initial value, input value will overlay placeholder after hydration.

I think it's reasonable because html is different between client side and server side.
Is there any solution for this behavior? Or should I just ignore it to make it no ssr?

codesandbox

Screenshot 2023-01-16 at 4 52 52 PM

@OliverJAsh
Copy link

once hydration does complete, the TextField reverts to blank, losing the user's edits, which is not good UX.

This issue contains a reduced test case against React 18: facebook/react#26974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: text field This is the name of the generic UI component, not the React module! new feature New feature or request package: material-ui Specific to @mui/material
Projects
None yet
Development

No branches or pull requests

6 participants