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.

Sign up for GitHub

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

Jump to bottom

fix: potential memory leaks when using nullish coalescing #29195

Closed
wants to merge 1 commit into from
Closed

fix: potential memory leaks when using nullish coalescing #29195

wants to merge 1 commit into from

Conversation

marcosmoura
Copy link
Contributor

@marcosmoura marcosmoura self-assigned this Sep 20, 2023
@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 634 609 5000
Button mount 321 316 5000
Field mount 1117 1093 5000
FluentProvider mount 687 685 5000
FluentProviderWithTheme mount 76 82 10
FluentProviderWithTheme virtual-rerender 65 57 10
FluentProviderWithTheme virtual-rerender-with-unmount 79 78 10
InfoButton mount 12 11 5000
MakeStyles mount 854 856 50000
Persona mount 1755 1723 5000
SpinButton mount 1392 1348 5000

@codesandbox-ci
Copy link

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 fa5e3a9:

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

@fabricteam
Copy link
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-accordion
Accordion (including children components)		 91.584 kB
27.965 kB		 91.577 kB
27.953 kB		 -7 B
-12 B
react-alert
Alert		 83.657 kB
22.895 kB		 83.647 kB
22.892 kB		 -10 B
-3 B
react-avatar
Avatar		 49.035 kB
15.434 kB		 49.028 kB
15.436 kB		 -7 B
2 B
react-avatar
AvatarGroupItem		 63.687 kB
19.786 kB		 63.684 kB
19.782 kB		 -3 B
-4 B
react-button
Button		 39.667 kB
10.794 kB		 39.657 kB
10.779 kB		 -10 B
-15 B
react-button
CompoundButton		 47.023 kB
12.295 kB		 47.012 kB
12.291 kB		 -11 B
-4 B
react-button
MenuButton		 44.094 kB
12.056 kB		 44.082 kB
12.049 kB		 -12 B
-7 B
react-button
SplitButton		 52.12 kB
13.632 kB		 52.119 kB
13.627 kB		 -1 B
-5 B
react-button
ToggleButton		 56.756 kB
12.702 kB		 56.745 kB
12.699 kB		 -11 B
-3 B
react-card
Card - All		 91.443 kB
26.437 kB		 91.442 kB
26.427 kB		 -1 B
-10 B
react-card
Card		 86.232 kB
24.895 kB		 86.226 kB
24.894 kB		 -6 B
-1 B
react-card
CardHeader		 14.431 kB
5.832 kB		 14.431 kB
5.833 kB
1 B
react-card
CardPreview		 13.097 kB
5.434 kB		 13.097 kB
5.433 kB
-1 B
react-components
react-components: Button, FluentProvider & webLightTheme		 69.572 kB
19.658 kB		 69.547 kB
19.658 kB		 -25 B
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover		 208.393 kB
59.395 kB		 208.345 kB
59.36 kB		 -48 B
-35 B
react-dialog
Dialog (including children components)		 90.125 kB
27.485 kB		 90.126 kB
27.479 kB		 1 B
-6 B
react-menu
Menu (including children components)		 139.823 kB
43.055 kB		 139.8 kB
43.044 kB		 -23 B
-11 B
react-menu
Menu (including selectable components)		 142.559 kB
43.576 kB		 142.536 kB
43.569 kB		 -23 B
-7 B
react-persona
Persona		 55.926 kB
17.314 kB		 55.919 kB
17.314 kB		 -7 B
react-spinner
Spinner		 22.481 kB
8.146 kB		 22.478 kB
8.142 kB		 -3 B
-4 B
react-table
DataGrid		 156.606 kB
43.603 kB		 156.603 kB
43.553 kB		 -3 B
-50 B
react-table
Table (Primitives only)		 42.651 kB
13.28 kB		 42.646 kB
13.268 kB		 -5 B
-12 B
react-table
Table as DataGrid		 129.472 kB
34.761 kB		 129.465 kB
34.749 kB		 -7 B
-12 B
react-table
Table (Selection only)		 74.748 kB
20.076 kB		 74.743 kB
20.061 kB		 -5 B
-15 B
react-table
Table (Sort only)		 73.379 kB
19.672 kB		 73.374 kB
19.655 kB		 -5 B
-17 B
react-tags-preview
InteractionTag		 14.038 kB
5.658 kB		 14.032 kB
5.652 kB		 -6 B
-6 B
react-tags-preview
Tag		 29.806 kB
9.597 kB		 29.804 kB
9.595 kB		 -2 B
-2 B
react-tags-preview
TagGroup		 72.687 kB
21.648 kB		 72.681 kB
21.647 kB		 -6 B
-1 B
react-toast
Toast (including Toaster)		 91.789 kB
27.423 kB		 91.764 kB
27.422 kB		 -25 B
-1 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-avatar
AvatarGroup		 18.782 kB
7.48 kB
react-card
CardFooter		 12.14 kB
5.061 kB
react-components
react-components: FluentProvider & webLightTheme		 40.966 kB
13.569 kB
react-portal-compat
PortalCompatProvider		 6.541 kB
2.227 kB
🤖 This report was generated against 48153fef6aa6ba63abfdfad25fb76dea13395b94

@marcosmoura marcosmoura changed the title fix: potential memory leak when using nullish coalescing fix: potential memory leaks when using nullish coalescing Sep 20, 2023
@size-auditor
Copy link

size-auditor bot commented Sep 20, 2023

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: c60d4e7

Possible causes

  • The baseline build c60d4e7 is broken
  • The Size Auditor run for the baseline build c60d4e7 was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

bsunderhus
bsunderhus reviewed Sep 20, 2023
View reviewed changes
Copy link
Contributor

@bsunderhus bsunderhus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we later on create an Issue to have an ESLint rule to enforce this?! I'm not sure how easy would it be to solve this on SWC side....

@Hotell
Copy link
Contributor

Hotell commented Sep 21, 2023

Should we later on create an Issue to have an ESLint rule to enforce this?! I'm not sure how easy would it be to solve this on SWC side....

swc already merged a fix, waiting for new release.

if you follow internal teams chanel you can get answers to your questions as well

@Hotell Hotell mentioned this pull request Sep 22, 2023
Merged
@marcosmoura
Copy link
Contributor Author

Closed as this issue was covered by SWC library

@marcosmoura marcosmoura deleted the fix/add-brackets-for-nullish-coalescing-operator branch November 15, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsunderhus bsunderhus bsunderhus left review comments

@behowell behowell behowell approved these changes

@layershifter layershifter Awaiting requested review from layershifter

@tomi-msft tomi-msft Awaiting requested review from tomi-msft

@khmakoto khmakoto Awaiting requested review from khmakoto khmakoto is a code owner

@sopranopillow sopranopillow Awaiting requested review from sopranopillow

Assignees

@marcosmoura marcosmoura

Labels
Type: Bug 🐛
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marcosmoura @fabricteam @Hotell @bsunderhus @behowell