Some code cleanup - Githubissues

aframevr / aframe-inspector

:mag: Visual inspector tool for A-Frame. Hit *<ctrl> + <alt> + i* on any A-Frame scene.

https://aframe.io/aframe-inspector/examples/

MIT License

654 stars 201 forks source link

Some code cleanup #639

Closed vincentfretin closed 1 year ago

vincentfretin commented 1 year ago

Remove dead code, unused imports, unused variables.
Use import/export everywhere, latest wepback/babel don't allow to use module.exports if you used one import statement.
Specify a prettier config with option singleQuote to match the current code. I didn't reformatted the files with prettier in this PR.
Disable the eslint space-before-function-paren rule that conflicts with how prettier format the files.
Fix eslint errors.

vincentfretin commented 1 year ago

eslint errors down to 15 errors instead of 350 errors

vincentfretin commented 1 year ago

I'll fix the remaining eslint errors later then this PR should be good to merge if there is no objection.

vincentfretin commented 1 year ago

I fixed all remaining eslint issues, excepted those: npm run lint

src/components/components/Mixins.js
  20:5  error  Do not use setState in componentDidUpdate  react/no-did-update-set-state

I'll fix this one as part of the react update in #641.

src/index.js
  104:11  error  'geometry' is defined but never used  no-unused-vars
  105:11  error  'material' is defined but never used  no-unused-vars

This is already fixed in #638

This PR is ready to be merged after #638.

vincentfretin commented 1 year ago

npm run prettier will reformat those files

    modifié :         src/components/Main.js
    modifié :         src/components/components/CommonComponents.js
    modifié :         src/components/components/Component.js
    modifié :         src/components/scenegraph/Toolbar.js
    modifié :         src/components/viewport/CameraToolbar.js
    modifié :         src/lib/EditorControls.js
    modifié :         src/lib/cameras.js
    modifié :         src/lib/raycaster.js
    modifié :         src/lib/viewport.js

but we can do that in a separate PR, I created the issue #642 for this.

dmarcos commented 1 year ago

This one is ready to go? Thanks for the patience.

vincentfretin commented 1 year ago

Yes, please merge.