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

chore(scene composer): addressing react-hook linter warnings #2797

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mumanity
Copy link
Contributor

@mumanity mumanity commented May 8, 2024

Overview

Addressing a portion of the linter warnings from the implementation of react-hook linter on Scene Composer. Takes us from 712 to 672 warnings.

Updates include:

  • Removing unused imports
  • Removing unused dependencies in hooks
  • Adding missing dependencies in hooks to prevent infinite loops & ensure accurate executions

Verifying Changes

Scene Composer

For scene-composer package changes specifically, you can preview the component in the published storybook artifact. To do this, wait for the Publish Storybook action to complete below.

  • Click on the workflow details
  • Select the Summary item on the left
  • Download the zip file

To run the storybook build locally, you need a local static web server:

npm install -g httpserver
cd <Extracted Zip Directory>
httpserver

Then open the website http://localhost:8080 to run the doc site.

Legal

This project is available under the Apache 2.0 License.

@mumanity mumanity force-pushed the react-hook-cleanup branch 5 times, most recently from 817a131 to 972ea37 Compare May 9, 2024 15:19
@mumanity mumanity marked this pull request as ready for review May 9, 2024 15:55
@@ -294,7 +310,7 @@ export function AsyncLoadedAnchorWidget({
lines.layers.disable(Layers.RaycastAndRender);
lines.layers.enable(Layers.RenderOnly);
}
}, [linesRef.current]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it necessary to wait for the ref to update?

@hwandersman
Copy link
Contributor

Did you manually update these or did you use the linter to auto-fix the warnings?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants