Skip to content

fix: Removing possible recursive loop in Coachmark #26934

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
Merged
spmonahan merged 7 commits into from
Feb 21, 2023
Merged

fix: Removing possible recursive loop in Coachmark #26934

spmonahan merged 7 commits into from
Feb 21, 2023

Conversation

@micahgodbolt
Copy link
Member

@micahgodbolt micahgodbolt commented Feb 21, 2023 •
edited
Loading

Don't fire useEffect unless a dependency changes.

fixes #26933

@micahgodbolt micahgodbolt requested review from a team as code owners February 21, 2023 19:17
@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 21, 2023 •
edited
Loading

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

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration
Infinite Loop Issue #26933

@size-auditor
Copy link

size-auditor bot commented Feb 21, 2023 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Coachmark 86.739 kB 86.78 kB ExceedsBaseline     41 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: 1184f85a5fce46d831c0a85f5e3c4ab76f87c6c4 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 21, 2023 •
edited
Loading

📊 Bundle size report

🤖 This report was generated against 1184f85a5fce46d831c0a85f5e3c4ab76f87c6c4

@fabricteam
Copy link
Collaborator

fabricteam commented Feb 21, 2023 •
edited
Loading

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 1097 1141 5000
Breadcrumb mount 3104 2984 1000
Checkbox mount 2671 2581 5000
CheckboxBase mount 2291 2398 5000
ChoiceGroup mount 4478 4321 5000
ComboBox mount 1261 1252 1000
CommandBar mount 9715 9890 1000
ContextualMenu mount 16790 16953 1000
DefaultButton mount 1365 1387 5000
DetailsRow mount 3488 3386 5000
DetailsRowFast mount 3348 3253 5000
DetailsRowNoStyles mount 3234 3328 5000
Dialog mount 3169 3093 1000
DocumentCardTitle mount 597 582 1000
Dropdown mount 3144 3213 5000
FocusTrapZone mount 1991 2061 5000
FocusZone mount 1836 1900 5000
GroupedList mount 51823 58054 2
GroupedList virtual-rerender 24491 24027 2
GroupedList virtual-rerender-with-unmount 92031 93814 2
GroupedListV2 mount 567 569 2
GroupedListV2 virtual-rerender 547 559 2
GroupedListV2 virtual-rerender-with-unmount 559 562 2
IconButton mount 1837 1916 5000
Label mount 731 732 5000
Layer mount 4071 4342 5000
Link mount 845 828 5000
MenuButton mount 1661 1672 5000
MessageBar mount 2289 2362 5000
Nav mount 3038 3285 1000
OverflowSet mount 1373 1260 5000
Panel mount 2443 2418 1000
Persona mount 1296 1313 1000
Pivot mount 1651 1685 1000
PrimaryButton mount 1533 1531 5000
Rating mount 6711 7037 5000
SearchBox mount 1540 1538 5000
Shimmer mount 2824 2887 5000
Slider mount 2140 2100 5000
SpinButton mount 4600 4483 5000
Spinner mount 819 774 5000
SplitButton mount 3102 2897 5000
Stack mount 850 825 5000
StackWithIntrinsicChildren mount 2220 2385 5000
StackWithTextChildren mount 4676 4728 5000
SwatchColorPicker mount 10424 10339 5000
TagPicker mount 2736 2445 5000
TeachingBubble mount 94947 93647 5000
Text mount 814 798 5000
TextField mount 1591 1604 5000
ThemeProvider mount 1434 1508 5000
ThemeProvider virtual-rerender 1091 1027 5000
ThemeProvider virtual-rerender-with-unmount 2198 2089 5000
Toggle mount 1125 1117 5000
buttonNative mount 542 549 5000

spmonahan and others added 2 commits February 21, 2023 20:34
@spmonahan
fix: refactor useGetBounds
58f3ce8 
Moves useGetBounds() to module scope so the hook is not recreated on every render.
This also ensures we're not relying on component scope to consume props.

Updates useGetBounds() to use useEffect()'s dependency array to prevent
continual re-renders.
@micahgodbolt
Merge pull request #2 from spmonahan/coachmark-fix-loop
fd75d4c 
fix: refactor useGetBounds
@khmakoto khmakoto changed the title Coachmark fix loop fix: Removing possible recursive loop in Coachmark Feb 21, 2023
@micahgodbolt micahgodbolt enabled auto-merge (squash) February 21, 2023 21:33
@khmakoto khmakoto disabled auto-merge February 21, 2023 22:20
@khmakoto khmakoto enabled auto-merge (squash) February 21, 2023 22:20
@micahgodbolt micahgodbolt enabled auto-merge (squash) February 21, 2023 23:01
@spmonahan spmonahan disabled auto-merge February 21, 2023 23:15
@spmonahan spmonahan enabled auto-merge (squash) February 21, 2023 23:15
@spmonahan spmonahan merged commit 89ac728 into microsoft:master Feb 21, 2023
@micahgodbolt micahgodbolt deleted the coachmark-fix-loop branch February 22, 2023 00:16
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Feb 24, 2023
@marcosmoura
Merge branch 'master' into feat/drawer-base-component
f9fb7e8 
* master: (93 commits)
  chore: migrate to jest 27 (microsoft#26835)
  chore: make lint task run without need of build (microsoft#26872)
  chore(react-table): exports UseTableSelectionOptions (microsoft#26892)
  applying package updates
  fix(react-card): allow elements to grow to fill the available space (microsoft#26616)
  fix: Popover without focus trap should not be aria-hidden (microsoft#26932)
  applying package updates
  applying package updates
  fix(react-combobox): Remove _getAriaActiveDescendantValue, compute aria-activedescendantvalue in state, and update currentPendingValue when the options change (microsoft#26574)
  fix: v8 Combobox role and accname for non-hidden icon button (microsoft#26905)
  fix: Removing possible recursive loop in Coachmark (microsoft#26934)
  Combobox: Fix cursor jumping to the end of input (microsoft#26931)
  Fix missing icons on website (microsoft#26797)
  fix: Fix the width of Input's focus border with appearance=underline (microsoft#26881)
  chore: Clean up Input's interactive styles (microsoft#26865)
  Remove codeowners from change files (microsoft#26935)
  chore: add splitbutton error warning to docs, remove button ariaDescription example (microsoft#26904)
  docs: Remove testing code from MenuList example (microsoft#26929)
  chore: refactor SpinButton to use makeResetStyles (microsoft#26867)
  feat: Set overflow on positioned element when `autosize` is applied (microsoft#26868)
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Mar 15, 2023
@marcosmoura
Merge branch 'feat/drawer-base-component' into marcosmoura/feat/drawe…
05a6c99 
…r-component

* feat/drawer-base-component: (141 commits)
  remove DrawerContainer
  feat: WIP add initial draft for Drawer and DrawerContainer
  chore: migrate to jest 27 (microsoft#26835)
  chore: make lint task run without need of build (microsoft#26872)
  chore(react-table): exports UseTableSelectionOptions (microsoft#26892)
  applying package updates
  fix(react-card): allow elements to grow to fill the available space (microsoft#26616)
  fix: Popover without focus trap should not be aria-hidden (microsoft#26932)
  applying package updates
  applying package updates
  fix(react-combobox): Remove _getAriaActiveDescendantValue, compute aria-activedescendantvalue in state, and update currentPendingValue when the options change (microsoft#26574)
  fix: v8 Combobox role and accname for non-hidden icon button (microsoft#26905)
  fix: Removing possible recursive loop in Coachmark (microsoft#26934)
  Combobox: Fix cursor jumping to the end of input (microsoft#26931)
  Fix missing icons on website (microsoft#26797)
  fix: Fix the width of Input's focus border with appearance=underline (microsoft#26881)
  chore: Clean up Input's interactive styles (microsoft#26865)
  Remove codeowners from change files (microsoft#26935)
  chore: add splitbutton error warning to docs, remove button ariaDescription example (microsoft#26904)
  docs: Remove testing code from MenuList example (microsoft#26929)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabricteam fabricteam fabricteam left review comments

@khmakoto khmakoto khmakoto approved these changes

Assignees

@Hotell Hotell

Labels

Component: Coachmark

Projects

None yet

Milestone

February Project Cycle Q1 2023

Development

Successfully merging this pull request may close these issues.

[Bug]: Coachmark creates an infinite React render loop

6 participants

@micahgodbolt @fabricteam @khmakoto @Hotell @msft-fluent-ui-bot @spmonahan