Skip to content

chore(react-menu): removes prop spreading on MenuList state#25672

Merged
bsunderhus merged 2 commits into
microsoft:masterfrom
bsunderhus:react-menu/chore--stop-prop-spreading-on-state
Dec 5, 2022
Merged

chore(react-menu): removes prop spreading on MenuList state#25672
bsunderhus merged 2 commits into
microsoft:masterfrom
bsunderhus:react-menu/chore--stop-prop-spreading-on-state

Conversation

@bsunderhus

@bsunderhus bsunderhus commented Nov 15, 2022

Copy link
Copy Markdown
Contributor

Main changes

  1. Removes the spreading of props into MenuList state declaration.
  2. Updates types accordingly, deprecating unnecessary properties passing down for the state
  3. deprecates unused type UninitializedMenuListState

This PR is a code maintenance

Together with type casting, spreading props at state is not ideal as it obscures what properties are actually required to maintain the proper usage of internal state, therefore spreading props at state should be avoided.

@bsunderhus bsunderhus force-pushed the react-menu/chore--stop-prop-spreading-on-state branch from a3daf61 to bff7b96 Compare November 15, 2022 11:16
@fabricteam

fabricteam commented Nov 15, 2022

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
190.427 kB
53.087 kB
190.235 kB
53.064 kB
-192 B
-23 B
react-menu
Menu (including children components)
117.61 kB
36.287 kB
117.418 kB
36.256 kB
-192 B
-31 B
react-menu
Menu (including selectable components)
120.679 kB
36.82 kB
120.487 kB
36.785 kB
-192 B
-35 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
62.951 kB
17.675 kB
react-components
react-components: FluentProvider & webLightTheme
33.456 kB
11.029 kB
react-portal-compat
PortalCompatProvider
5.857 kB
1.978 kB
🤖 This report was generated against 71139fbfe025c22f9934738ab53d14b5f62e4d66

@fabricteam

fabricteam commented Nov 15, 2022

Copy link
Copy Markdown
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1557 1530 5000
Button mount 1038 1060 5000
FluentProvider mount 1775 1770 5000
FluentProviderWithTheme mount 653 655 10
FluentProviderWithTheme virtual-rerender 612 602 10
FluentProviderWithTheme virtual-rerender-with-unmount 646 655 10
MakeStyles mount 2062 2095 50000
SpinButton mount 3026 3019 5000

@size-auditor

size-auditor Bot commented Nov 15, 2022

Copy link
Copy Markdown

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 71139fbfe025c22f9934738ab53d14b5f62e4d66 (build)

@codesandbox-ci

codesandbox-ci Bot commented Nov 15, 2022

Copy link
Copy Markdown

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 5f5cb25:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@bsunderhus bsunderhus force-pushed the react-menu/chore--stop-prop-spreading-on-state branch from e37dd71 to 5f5cb25 Compare November 15, 2022 11:47
@bsunderhus bsunderhus marked this pull request as ready for review November 15, 2022 12:07
@bsunderhus bsunderhus requested review from a team as code owners November 15, 2022 12:07
@bsunderhus bsunderhus merged commit 41b0dbe into microsoft:master Dec 5, 2022
@bsunderhus bsunderhus deleted the react-menu/chore--stop-prop-spreading-on-state branch December 5, 2022 09:51
ling1726 added a commit to ling1726/fluentui that referenced this pull request Jan 9, 2023
Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #
ling1726 added a commit that referenced this pull request Jan 9, 2023
* fix: MenuList props should win over context props

Fixes the regression introduced in #25672 so that prop values win over
context props with the same name.

Fixes #

* changefile
q1b pushed a commit to q1b/fluentui that referenced this pull request Jan 24, 2023
* fix: MenuList props should win over context props

Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #

* changefile
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
…t#25672)

* chore(react-menu): removes prop spreading on MenuList state

* chore: deprecates unused type UninitializedMenuListState
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
* fix: MenuList props should win over context props

Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #

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

3 participants