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

Strange difference in behaviour between preact and react in combination with @material-ui and conditional operator #2949

Closed
masbaehr opened this issue Jan 21, 2021 · 4 comments · Fixed by #4550
Labels

Comments

@masbaehr
Copy link

This is the code to test the behavior difference.

Note: I know that it can be fixed by using the "boolswitch &&" / "!boolswitch &&" Syntax. But i noticed this after switching from react to preact. I'm not entirely sure if this might be a material-ui shortcoming but as it happened after switching to Preact i thought it might be better suited to post the issue here.

import React from 'react';
import TextField from '@material-ui/core/TextField';


function Debug(props) {
 
  const [boolswitch, setboolswitch] = React.useState(false);

  return (
    <div>
        
    {boolswitch ? '' : 
        
        <TextField  
          style={{width: "100%"}}
          label={"Username"}
          variant="outlined"
          margin="normal"
          onKeyPress={event => {

          }}
          onChange={event => {}}
        />
    }
      {boolswitch ? '' : 
         <TextField  
          style={{width: "100%"}}
          label={"Password"}
          variant="outlined"
          margin="normal"
          onKeyPress={event => {

          }}
          onChange={event => {}}
        />
        
       
    }
    {boolswitch ?
     
        <TextField  
          style={{width: "100%"}}
          label={"2fa"}
          variant="outlined"
          margin="normal"
          onKeyPress={event => {

          }}
          onChange={event => {}}
        />
        : ''
    }
      <button onClick={event => {
          setboolswitch(true);
      }}>Continue</button>
    </div>
  );
}

export default Debug;

First step:

image

Second step:
Problem: The internal value state of the username field is rendered for the now visible 2fa field

image

I also know that it could be avoided using States for all the values, but this would mean that a state update while typing would be needed which slows down the typing.

Summary: Somehow the ? '' : '' Syntax seems to cause a difference while rendering in Preact vs. React

@developit
Copy link
Member

@masbaehr This can be corrected by adding key="2fa" to the last TextField.

@masbaehr
Copy link
Author

Oh okay, is it a requirement in preact to use "key" for every component? Maybe I missed that? By the way i also noticed some other differences in rendering e.g. the defaultValue is not correctly populated if a variable changes on rerender

e.g. for a wrapped in { loadingdone && <Textfield defaultValue={value}} it was remaining empty even if value is set and loadingdone true.

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Jan 22, 2021

@masbaehr No it's not a requirement. Keys can certainly give hints to the reconciler, but the behaviour described here is a bug. Keys do serve as a workaround here if you urgently need to ship it into production.

Let's file a separate issue for the defaultValue issue you're experiencing. Otherwise it becomes a nightmare for us to track and prioritise👍

@marvinhagemeister
Copy link
Member

Managed to narrow it down to a simple reproduction case:

// #2949
it('should not swap unkeyed chlildren', () => {
	class X extends Component {
		constructor(props) {
			super(props);
			this.name = props.name;
		}
		render() {
			return <p>{this.name}</p>;
		}
	}

	function Foo({ condition }) {
		return (
			<div>
				{condition ? '' : <X name="A" />}
				{condition ? <X name="B" /> : ''}
			</div>
		);
	}

	render(<Foo />, scratch);
	expect(scratch.textContent).to.equal('A');

	render(<Foo condition />, scratch);
	expect(scratch.textContent).to.equal('B');

	render(<Foo />, scratch);
	expect(scratch.textContent).to.equal('A');
});

Interestingly the error doesn't occur when we replace the strings as an empty value with null.

JoviDeCroock added a commit that referenced this issue Nov 10, 2024
With keyed children we can confidently track them as they move
throughout an array of components. When this happens with unkeyed
functional components we should not risk reusing the state as that
could possibly make us re-use state in unrelated components, as
seen in #2949.
JoviDeCroock added a commit that referenced this issue Nov 10, 2024
With keyed children we can confidently track them as they move
throughout an array of components. When this happens with unkeyed
functional components we should not risk reusing the state as that
could possibly make us re-use state in unrelated components, as
seen in #2949.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants