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

Cleanup env variables #24739

Merged
merged 19 commits into from
Sep 14, 2022
Merged

Cleanup env variables #24739

merged 19 commits into from
Sep 14, 2022

Conversation

ralucapelin
Copy link
Contributor

Current Behavior

Multiple environment variables that are declared are not necessary.

New Behavior

The redundant environment variables are removed.

Related Issue(s)

Fixes #

@ralucapelin ralucapelin added Area: Build System Ready for VR Used to trigger screener checks for PRs labels Sep 9, 2022
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 9, 2022

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

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

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 9, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-dialog
Dialog (including children components)
80.608 kB
24.053 kB
81.048 kB
24.208 kB
440 B
155 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
global-context
createContext
533 B
341 B
global-context
createContextSelector
554 B
348 B
priority-overflow
createOverflowManager
2.936 kB
1.212 kB
react-accordion
Accordion (including children components)
78.914 kB
24.06 kB
react-alert
Alert
83.511 kB
20.921 kB
react-avatar
Avatar
48.381 kB
13.696 kB
react-avatar
AvatarGroup
14.95 kB
5.989 kB
react-avatar
AvatarGroupItem
68.349 kB
19.039 kB
react-badge
Badge
22.6 kB
7.205 kB
react-badge
CounterBadge
23.503 kB
7.497 kB
react-badge
PresenceBadge
24.05 kB
7.067 kB
react-button
Button
36.119 kB
9.647 kB
react-button
CompoundButton
43.144 kB
10.86 kB
react-button
MenuButton
38.737 kB
10.521 kB
react-button
SplitButton
46.136 kB
11.9 kB
react-button
ToggleButton
51.888 kB
11.127 kB
react-card
Card - All
67.002 kB
19.261 kB
react-card
Card
62.684 kB
18.177 kB
react-card
CardFooter
8.561 kB
3.601 kB
react-card
CardHeader
9.604 kB
3.94 kB
react-card
CardPreview
8.662 kB
3.656 kB
react-combobox
Combobox (including child components)
74.636 kB
24.186 kB
react-combobox
Dropdown (including child components)
74.236 kB
24.086 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
187.939 kB
52.05 kB
react-components
react-components: FluentProvider & webLightTheme
33.359 kB
11.004 kB
react-divider
Divider
16.459 kB
5.902 kB
react-image
Image
10.78 kB
4.264 kB
react-input
Input
23.598 kB
7.661 kB
react-label
Label
9.338 kB
3.86 kB
react-link
Link
11.784 kB
4.867 kB
react-menu
Menu (including children components)
115.735 kB
35.419 kB
react-menu
Menu (including selectable components)
118.934 kB
35.916 kB
react-overflow
hooks only
10.685 kB
4.104 kB
react-popover
Popover
102.938 kB
31.542 kB
react-portal
Portal
10.576 kB
3.875 kB
react-portal-compat
PortalCompatProvider
5.851 kB
1.964 kB
react-positioning
usePositioning
19.7 kB
7.404 kB
react-provider
FluentProvider
15.755 kB
5.883 kB
react-radio
Radio
35.56 kB
11.929 kB
react-radio
RadioGroup
14.248 kB
5.7 kB
react-select
Select
20.846 kB
7.346 kB
react-slider
Slider
31.526 kB
10.046 kB
react-spinbutton
SpinButton
43.943 kB
12.382 kB
react-spinner
Spinner
19.977 kB
6.438 kB
react-switch
Switch
32.097 kB
10.27 kB
react-text
Text - Default
11.782 kB
4.605 kB
react-text
Text - Wrappers
15.092 kB
5.044 kB
react-textarea
Textarea
23.988 kB
8.011 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
29.548 kB
6.434 kB
react-theme
Teams: Light theme
17.452 kB
5.054 kB
react-tooltip
Tooltip
41.502 kB
14.623 kB
react-utilities
SSRProvider
180 B
159 B
🤖 This report was generated against b51ebaec4a0681dc71e8f67a28a9d28b051bcba8

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 9, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1269 1218 5000
Button mount 836 848 5000
FluentProvider mount 1479 1496 5000
FluentProviderWithTheme mount 565 609 10
FluentProviderWithTheme virtual-rerender 516 576 10
FluentProviderWithTheme virtual-rerender-with-unmount 630 612 10
MakeStyles mount 1926 1885 50000
SpinButton mount 2456 2507 5000

@size-auditor
Copy link

size-auditor bot commented Sep 9, 2022

Asset size changes

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

Baseline commit: fd88d8b10fe32eb635c949aa28653c3376f64c3d (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 9, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
TreeWith60ListItems.default 147 129 1.14:1
ChatDuplicateMessagesPerf.default 235 220 1.07:1
ChatWithPopoverPerf.default 301 283 1.06:1
PortalMinimalPerf.default 144 136 1.06:1
ButtonMinimalPerf.default 143 136 1.05:1
SkeletonMinimalPerf.default 314 299 1.05:1
AvatarMinimalPerf.default 168 161 1.04:1
ListNestedPerf.default 473 457 1.04:1
ListWith60ListItems.default 491 471 1.04:1
CardMinimalPerf.default 469 456 1.03:1
FlexMinimalPerf.default 249 242 1.03:1
RefMinimalPerf.default 185 180 1.03:1
SegmentMinimalPerf.default 303 295 1.03:1
DropdownManyItemsPerf.default 535 525 1.02:1
InputMinimalPerf.default 858 839 1.02:1
RosterPerf.default 1722 1691 1.02:1
TextAreaMinimalPerf.default 414 406 1.02:1
AnimationMinimalPerf.default 471 466 1.01:1
EmbedMinimalPerf.default 2628 2610 1.01:1
GridMinimalPerf.default 296 293 1.01:1
HeaderSlotsPerf.default 676 668 1.01:1
ItemLayoutMinimalPerf.default 976 969 1.01:1
LabelMinimalPerf.default 337 334 1.01:1
PopupMinimalPerf.default 556 553 1.01:1
ProviderMergeThemesPerf.default 989 979 1.01:1
RadioGroupMinimalPerf.default 387 383 1.01:1
SliderMinimalPerf.default 1236 1221 1.01:1
CustomToolbarPrototype.default 2156 2145 1.01:1
ToolbarMinimalPerf.default 795 790 1.01:1
ButtonOverridesMissPerf.default 1006 1011 1:1
ButtonSlotsPerf.default 423 421 1:1
ChatMinimalPerf.default 628 630 1:1
DropdownMinimalPerf.default 2164 2156 1:1
FormMinimalPerf.default 320 320 1:1
ListMinimalPerf.default 450 451 1:1
LoaderMinimalPerf.default 516 514 1:1
MenuMinimalPerf.default 733 733 1:1
MenuButtonMinimalPerf.default 1356 1355 1:1
SplitButtonMinimalPerf.default 3279 3270 1:1
StatusMinimalPerf.default 600 601 1:1
IconMinimalPerf.default 562 560 1:1
TableManyItemsPerf.default 1566 1573 1:1
TextMinimalPerf.default 298 297 1:1
BoxMinimalPerf.default 293 297 0.99:1
CarouselMinimalPerf.default 345 350 0.99:1
CheckboxMinimalPerf.default 1522 1537 0.99:1
DialogMinimalPerf.default 680 687 0.99:1
HeaderMinimalPerf.default 313 315 0.99:1
LayoutMinimalPerf.default 309 311 0.99:1
ProviderMinimalPerf.default 312 314 0.99:1
ReactionMinimalPerf.default 328 330 0.99:1
TableMinimalPerf.default 347 350 0.99:1
TooltipMinimalPerf.default 1852 1879 0.99:1
AttachmentMinimalPerf.default 124 127 0.98:1
AttachmentSlotsPerf.default 852 871 0.98:1
DividerMinimalPerf.default 306 311 0.98:1
ImageMinimalPerf.default 338 346 0.98:1
TreeMinimalPerf.default 679 695 0.98:1
DatepickerMinimalPerf.default 4548 4676 0.97:1
ListCommonPerf.default 501 519 0.97:1
VideoMinimalPerf.default 598 625 0.96:1
AlertMinimalPerf.default 215 231 0.93:1
AccordionMinimalPerf.default 112 127 0.88:1

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 9, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 912 904 5000
Breadcrumb mount 2903 2866 1000
Checkbox mount 2605 2640 5000
CheckboxBase mount 2256 2310 5000
ChoiceGroup mount 4622 4615 5000
ComboBox mount 971 983 1000
CommandBar mount 10724 10591 1000
ContextualMenu mount 12003 12253 1000
DefaultButton mount 1120 1123 5000
DetailsRow mount 3675 3744 5000
DetailsRowFast mount 3665 3715 5000
DetailsRowNoStyles mount 3493 3548 5000
Dialog mount 3052 3061 1000
DocumentCardTitle mount 165 164 1000
Dropdown mount 3240 3232 5000
FocusTrapZone mount 1831 1835 5000
FocusZone mount 1788 1739 5000
IconButton mount 1683 1675 5000
Label mount 358 355 5000
Layer mount 4401 4456 5000
Link mount 457 456 5000
MenuButton mount 1446 1463 5000
MessageBar mount 2168 2141 5000
Nav mount 3254 3285 1000
OverflowSet mount 1120 1103 5000
Panel mount 2405 2456 1000
Persona mount 954 971 1000
Pivot mount 1406 1390 1000
PrimaryButton mount 1249 1262 5000
Rating mount 7611 7542 5000
SearchBox mount 1260 1263 5000
Shimmer mount 2765 2839 5000
Slider mount 1942 1935 5000
SpinButton mount 4917 4861 5000
Spinner mount 433 428 5000
SplitButton mount 3147 3071 5000
Stack mount 514 509 5000
StackWithIntrinsicChildren mount 2268 2260 5000
StackWithTextChildren mount 5104 5115 5000
SwatchColorPicker mount 11923 11650 5000
TagPicker mount 2808 2720 5000
TeachingBubble mount 97599 99748 5000
Text mount 440 437 5000
TextField mount 1384 1383 5000
ThemeProvider mount 1329 1325 5000
ThemeProvider virtual-rerender 803 777 5000
ThemeProvider virtual-rerender-with-unmount 2108 2109 5000
Toggle mount 805 840 5000
buttonNative mount 135 138 5000

@ralucapelin ralucapelin marked this pull request as ready for review September 13, 2022 07:07
@ralucapelin ralucapelin requested review from a team as code owners September 13, 2022 07:07
* @returns
*/
function getConfig(SCREENER_API_KEY, BUILD_SOURCEBRANCHNAME, DEPLOYURL, SYSTEM_PULLREQUEST_TARGETBRANCH) {
const baseBranch = SYSTEM_PULLREQUEST_TARGETBRANCH
Copy link
Member

Choose a reason for hiding this comment

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

does the target branch still include ref/heads ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have yet to see another value for this env variable other than master but I left it in the case the target branch would be different

scripts/config.ts Outdated Show resolved Hide resolved
apps/vr-tests-react-components/screener.config.js Outdated Show resolved Hide resolved
*/
function getConfig(screenerApiKey, sourceBranchName, deployUrl, targetBranch) {
const baseBranch = targetBranch ? targetBranch.replace(/^refs\/heads\//, '') : 'master';
// https://github.com/screener-io/screener-storybook#additional-configuration-options
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be great if we can extract this type to encapsulated interface and use it whenever needed

apps/vr-tests/screener.config.js Outdated Show resolved Hide resolved
baseBranch,
failureExitCode: 0,
alwaysAcceptBaseBranch: true,
...(sourceBranchName !== 'master' ? { commit: getCurrentHash() } : null),
Copy link
Contributor

Choose a reason for hiding this comment

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

there is duplication in every config defined, can we make it DRY ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the configs for v8 and v9 are indeed almost identical, will check if it's possible to use only one file

Copy link
Contributor

Choose a reason for hiding this comment

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

they should live within scripts/screener domain, later on this will be separate package

Copy link
Contributor Author

@ralucapelin ralucapelin Sep 13, 2022

Choose a reason for hiding this comment

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

will make this change in the same follow-up PR as the one mentioned here as this PR only deals with changes regarding env variables: https://github.com/microsoft/fluentui/pull/24739/files#r969406141

@ralucapelin ralucapelin merged commit f83c3a1 into master Sep 14, 2022
@ralucapelin ralucapelin deleted the cleanup-env-variables branch September 14, 2022 13:40
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 14, 2022
* master: (28 commits)
  Fix value font-weight inside heatmap chart (microsoft#24726)
  Fix legend overflow-indication-text role (microsoft#24756)
  Support custom locale in date axis  (microsoft#24753)
  Cleanup env variables (microsoft#24739)
  ci(github): add GH Action to add issue labels based on new GH issue template (microsoft#24788)
  Update disallowedChangeTypes for newly created packages, to allow only 'prerelease' change types by default (microsoft#24763)
  feat(react-components): Adding missing AvatarGroup exports (microsoft#24770)
  remove unnecessary nohoist (microsoft#24760)
  feat(react-dialog): supports 1st rule of ARIA (microsoft#24525)
  BREAKING: TableCell layouts are handled by layout components (microsoft#24762)
  feat: Implement table cell layout components (microsoft#24773)
  applying package updates
  fix: remove readonly from DetailsList (microsoft#24615)
  chore: Cleaning up tokens in Button components so they better adhere to the design spec (microsoft#24732)
  fix: react-combobox listbox popup width matches trigger width (microsoft#24733)
  fix: react-combobox Option focus outline only shows with keyboard nav (microsoft#24700)
  feat: Publish react-field package, and export from react-components/unstable (microsoft#24235)
  fix: Replacing bottom border styles with text decoration underline in Link (microsoft#24734)
  docs(react-theme): Update readme (microsoft#24755)
  Add tests for hover states (microsoft#24390)
  ...
@ling1726 ling1726 mentioned this pull request Oct 11, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Build System Ready for VR Used to trigger screener checks for PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants