Skip to content

Replace anonymous useEffect functions with named functions#553

Merged
Zhou-Ziheng merged 5 commits intodeephaven:mainfrom
Zhou-Ziheng:532-replace-anonymous-functions
May 9, 2022
Merged

Replace anonymous useEffect functions with named functions#553
Zhou-Ziheng merged 5 commits intodeephaven:mainfrom
Zhou-Ziheng:532-replace-anonymous-functions

Conversation

@Zhou-Ziheng
Copy link
Contributor

Fixes #532

@Zhou-Ziheng Zhou-Ziheng added the enhancement New feature or request label May 5, 2022
@mofojed
Copy link
Member

mofojed commented May 6, 2022

Your tests are failing because of an eslint rule prefer-arrow-callback. You can disable that rule in our eslint config:

'no-useless-constructor': 'off',

Just add a 'prefer-arrow-callback': 'off' line there with a comment

@codecov
Copy link

codecov bot commented May 6, 2022

Codecov Report

Merging #553 (2eb5700) into main (a1f25a6) will increase coverage by 0.06%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #553      +/-   ##
==========================================
+ Coverage   36.37%   36.43%   +0.06%     
==========================================
  Files         390      390              
  Lines       28577    28642      +65     
  Branches     6774     6777       +3     
==========================================
+ Hits        10394    10436      +42     
- Misses      17837    17860      +23     
  Partials      346      346              
Flag Coverage Δ
unit 36.43% <50.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/code-studio/src/main/AppControlsMenu.jsx 11.47% <0.00%> (-0.20%) ⬇️
packages/code-studio/src/main/AppInit.jsx 0.00% <0.00%> (ø)
packages/code-studio/src/settings/ShortcutItem.tsx 0.00% <0.00%> (ø)
packages/components/src/AutoResizeTextarea.tsx 0.00% <0.00%> (ø)
packages/components/src/CardFlip.tsx 0.00% <0.00%> (ø)
packages/components/src/Checkbox.tsx 0.00% <0.00%> (ø)
packages/file-explorer/src/FileExplorer.tsx 0.00% <0.00%> (ø)
packages/file-explorer/src/FileList.tsx 10.09% <0.00%> (-0.15%) ⬇️
packages/file-explorer/src/FileListItemEditor.tsx 0.00% <0.00%> (ø)
...ebar/conditional-formatting/ColumnFormatEditor.tsx 5.88% <0.00%> (-0.12%) ⬇️
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1f25a6...2eb5700. Read the comment docs.

@Zhou-Ziheng Zhou-Ziheng marked this pull request as ready for review May 6, 2022 17:53
@mattrunyon
Copy link
Collaborator

Should probably change the eslint rule to just allowNamedFunctions instead of removing it entirely. Still prefer arrows over unnamed functions

https://eslint.org/docs/rules/prefer-arrow-callback#allownamedfunctions

@Zhou-Ziheng Zhou-Ziheng self-assigned this May 9, 2022
@Zhou-Ziheng Zhou-Ziheng requested a review from mofojed May 9, 2022 13:21
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Rebase off the latest main as well. Let me know if you need help stepping through that.

}
);
useEffect(
function updateViewportAndReturnClaenup() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function updateViewportAndReturnClaenup() {
function updateViewportAndReturnCleanup() {

@Zhou-Ziheng Zhou-Ziheng force-pushed the 532-replace-anonymous-functions branch from 5386148 to 0cc2176 Compare May 9, 2022 15:32
@Zhou-Ziheng Zhou-Ziheng requested a review from mofojed May 9, 2022 15:37
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Still need to update the typo in StorageTableViewportUpdater

@Zhou-Ziheng Zhou-Ziheng requested a review from mofojed May 9, 2022 16:56
@Zhou-Ziheng Zhou-Ziheng merged commit ecfc74a into deephaven:main May 9, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace anonymous useEffect functions with named functions

3 participants