Skip to content

Revert "fix: Input should start with undefined, same as textarea"#30845

Merged
ling1726 merged 3 commits intomasterfrom
revert-30534-master
Mar 25, 2024
Merged

Revert "fix: Input should start with undefined, same as textarea"#30845
ling1726 merged 3 commits intomasterfrom
revert-30534-master

Conversation

@ling1726
Copy link
Contributor

@ling1726 ling1726 commented Mar 21, 2024

Reverts #30534

The initial value of the input's controllable state is explicitly an empty string since we pass the value in to the native rendered <input /> always. After the above PR an uncontrolled Input component always renders a react warning because any user input will output this error in non-production environments

This is a blocking issue for any user who consumes this change and has test failures on console.error. Here's a screen recording

input error

If an undefined value is necessary as an initial value then we should create an issue to track since the change in non-trivial. We always pass the value to intrinsic input elements and that would need to change first.

@ling1726 ling1726 marked this pull request as ready for review March 21, 2024 12:24
@ling1726 ling1726 requested review from a team and spmonahan as code owners March 21, 2024 12:24
@fabricteam
Copy link
Collaborator

fabricteam commented Mar 21, 2024

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 56 61 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 636 635 5000
Button mount 306 301 5000
Field mount 1120 1161 5000
FluentProvider mount 698 696 5000
FluentProviderWithTheme mount 73 82 10
FluentProviderWithTheme virtual-rerender 56 61 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 70 81 10
MakeStyles mount 851 863 50000
Persona mount 1740 1716 5000
SpinButton mount 1377 1349 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 21, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-datepicker-compat
DatePicker Compat
225.758 kB
63.183 kB
225.754 kB
63.183 kB
-4 B
react-input
Input
28.126 kB
9.362 kB
28.122 kB
9.36 kB
-4 B
-2 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
71.098 kB
20.515 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
220.047 kB
62.171 kB
react-components
react-components: FluentProvider & webLightTheme
43.585 kB
14.352 kB
react-portal-compat
PortalCompatProvider
7.944 kB
2.588 kB
🤖 This report was generated against b0d5967aae4d30ed75a1ec6aa72437047a706688

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 21, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@ling1726 ling1726 closed this Mar 21, 2024
@ling1726 ling1726 reopened this Mar 21, 2024
@ling1726 ling1726 closed this Mar 21, 2024
@ling1726 ling1726 reopened this Mar 21, 2024
@ling1726 ling1726 closed this Mar 22, 2024
@ling1726 ling1726 reopened this Mar 22, 2024
@bsunderhus
Copy link
Contributor

bsunderhus commented Mar 22, 2024

If an undefined value is necessary as an initial value then we should create an issue to track since the change in non-trivial. We always pass the value to intrinsic input elements and that would need to change first.

This does sound like the better alternative IMO 👀, maybe we should investigate in stopping controlling the native input element if the user is not controlling it.

This is unrelated to this PR though, I'm just giving my 2 cents contribution here. I believe we should follow the controlled/uncontrolled pattern - @bsunderhus

@ling1726 ling1726 closed this Mar 25, 2024
@ling1726 ling1726 reopened this Mar 25, 2024
@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

@ling1726 ling1726 merged commit 2526d65 into master Mar 25, 2024
robertpenner pushed a commit to robertpenner/fluentui that referenced this pull request Apr 11, 2024
@khmakoto khmakoto deleted the revert-30534-master branch April 22, 2024 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants