-
Notifications
You must be signed in to change notification settings - Fork 2.9k
chore(react-utilities): simplifies useControllableState hook internals #27702
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
chore(react-utilities): simplifies useControllableState hook internals #27702
Conversation
|
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. Latest deployment of this branch, based on commit df609f8:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 37e89b9319db7f31f3afe0d38fc31eac40cb5eaf (build) |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| InfoButton | mount | 22 | 19 | 5000 | Possible regression |
All results
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 824 | 841 | 5000 | |
| Button | mount | 454 | 434 | 5000 | |
| Field | mount | 1507 | 1566 | 5000 | |
| FluentProvider | mount | 1073 | 1079 | 5000 | |
| FluentProviderWithTheme | mount | 133 | 132 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 110 | 118 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 123 | 116 | 10 | |
| InfoButton | mount | 22 | 19 | 5000 | Possible regression |
| MakeStyles | mount | 1307 | 1318 | 50000 | |
| Persona | mount | 2444 | 2364 | 5000 | |
| SpinButton | mount | 1870 | 1856 | 5000 |
🕵 fluentuiv9 No visual regressions between this PR and main |
|
@bsunderhus I think this PR also fixes #25748, would you be able to check? |
| * @returns - whether the value is controlled | ||
| */ | ||
| const useIsControlled = (controlledValue: unknown) => { | ||
| const useIsControlled = <V>(controlledValue?: V): controlledValue is V => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: value should be always passed
const useIsControlled = <V>(controlledValue: V | undefined): controlledValue is V => {
| const initialState = typeof options.defaultState === 'undefined' ? options.initialState : options.defaultState; | ||
| const [internalState, setInternalState] = React.useState<State>(initialState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more improvement might to move the check to factory, so it will be executed once:
const [internalState, setInternalState] = React.useState<State>(
() => typeof options.defaultState === 'undefined' ? options.initialState : options.defaultState
);* master: chore: enforce files naming to use .styles.ts [cxe-prg files] (microsoft#27707) chore: migrate to NodeJS v16 (microsoft#27711) remove react-avatar-context package (microsoft#27709) test: migrate all v9 libraries to use new conformance setup which improves test execution by approx 46% (microsoft#27669) chore: Add caret to react-datepicker-compat dependencies (microsoft#27671) test(fluentui/react): fix initial global leaks that were causing OOM issues (microsoft#27661) chore(react-utilities): simplifies useControllableState hook internals (microsoft#27702) chore: enforce files naming to use .styles.ts (microsoft#27698) feat: adds lazy loading example (microsoft#27587) feat: react-combobox space conditionally inserts character when freeform is true (microsoft#27025) chore(react-tree): updates useOpenItemsState internals (microsoft#27697) feat(tools): add conformance setup migration to migrate-converged-pkg generator (microsoft#27668) test: use `isolatedModules` for all ts-jest configs to lower memory footprint on CI (microsoft#27670) chore: bump @griffel/eslint-plugin & enable @griffel/hook-naming (microsoft#27687) feat(react-tree): makes useFlatTree generic (microsoft#27682) Enable export to codepen for charting library (microsoft#27539) Overflow: added useOverflowCount to exports (microsoft#27678) fix: Menu should not steal focus on re-render (microsoft#27688) feat(react-conformance): add new TS config api to be able to specify configName and configDir (microsoft#27664)
* feat/drawer-components: (28 commits) docs: update API feat: add style overrides for upcoming drawer components feat: add support to override drawer styles fix: prevent elements from stretching chore: upgrade devcontainer node version to 16 (microsoft#27716) fix: rename styles file to use the newer naming conventions test: add cypress tests for scroll positions chore: enforce files naming to use .styles.ts [cxe-prg files] (microsoft#27707) feat: add support to override drawer body styles chore: migrate to NodeJS v16 (microsoft#27711) remove react-avatar-context package (microsoft#27709) test: migrate all v9 libraries to use new conformance setup which improves test execution by approx 46% (microsoft#27669) chore: Add caret to react-datepicker-compat dependencies (microsoft#27671) test(fluentui/react): fix initial global leaks that were causing OOM issues (microsoft#27661) chore(react-utilities): simplifies useControllableState hook internals (microsoft#27702) chore: enforce files naming to use .styles.ts (microsoft#27698) feat: adds lazy loading example (microsoft#27587) feat: react-combobox space conditionally inserts character when freeform is true (microsoft#27025) chore(react-tree): updates useOpenItemsState internals (microsoft#27697) feat(tools): add conformance setup migration to migrate-converged-pkg generator (microsoft#27668) ...
|
🎉 Handy links: |


New Behavior
useControllableStateimplementation but ensuring same functionality according to tests