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

Update all dependencies and fix react code warnings #647

Merged
merged 14 commits into from
Dec 8, 2022

Conversation

vincentfretin
Copy link
Contributor

@vincentfretin vincentfretin commented Oct 9, 2022

Update all dependencies and fix react code warnings

This closes #641 and #601

Remaining todos:

  • copy html to clipboard broken after the upgrade
  • opening the textures modal doesn't work after the upgrade

@vincentfretin
Copy link
Contributor Author

gm, invariant, object-assign, react-file-reader-input, eslint-plugin-promise, postcss-import were unused dependencies
eslint-plugin-standard is not needed anymore with latest eslint-config-standard
babel-core is replaced by @babel/core
babel-eslint is replaced by @babel/eslint-parser
babel-preset-env is replaced by @babel/preset-env
babel-preset-react is replaced by @babel/preset-react
babel-plugin-transform-class-properties is included as part of @babel/preset-env
terser-webpack-plugin-legacy removed, terser-webpack-plugin is a dependency of webpack 5, and enabled automatically when using mode: production.

@vincentfretin
Copy link
Contributor Author

vincentfretin commented Oct 10, 2022

I rebased and push forced to add the fixes from #648
If you want to test, I pushed the build in my code-cleanup-plus-react-update-with-dist branch, it's the same commits as code-cleanup-plus-react-update but with an additional commit to push the build

<a-scene inspector="url:https://cdn.jsdelivr.net/gh/vincentfretin/aframe-inspector@f89a0b44190b1591d2d82f6f47ed7eecf4ef9b38/dist/aframe-inspector.min.js"

@vincentfretin
Copy link
Contributor Author

Copy html to clipboard and opening the textures modal doesn't work. I'll look into it later.

@vincentfretin
Copy link
Contributor Author

<a-scene inspector="url:https://cdn.jsdelivr.net/gh/vincentfretin/aframe-inspector@96b9ec69b4f1b5579cfd8d06d4b7971df64ace3a/dist/aframe-inspector.min.js"

includes the fix for #638 (comment)

@vincentfretin
Copy link
Contributor Author

Now using GLTFExporter directly from three package instead of the old one in vendor folder.

@dmarcos
Copy link
Member

dmarcos commented Dec 6, 2022

This needs rebase

@dmarcos
Copy link
Member

dmarcos commented Dec 6, 2022

Is it possible to break this down in smaller PRs?

@vincentfretin
Copy link
Contributor Author

Now that all the other PRs are merged, this one contains only the big update of react and all dependencies. I could split the last commit in its own PR where I add husky but currently it depends on another commit that changes package-lock.json, that would be a headache to merge this commit first then rebase this PR. I can separate it when I'm done fixing the regression with the clipboard.

@dmarcos
Copy link
Member

dmarcos commented Dec 7, 2022

38k lines the package-lock.json file? Is not that a bit crazy 😄? Can it be purged / simplified? Adds tons of noise

@vincentfretin
Copy link
Contributor Author

Ah ah no it can't be simplified, we need to commit the file as is.
It's just all the dependencies that are installed in a typical react app. The file is needed to install exactly those versions when you run npm install.
The package-lock.json in aframe repo is 25k lines. :)

@dmarcos dmarcos merged commit 24b9a72 into aframevr:master Dec 8, 2022
@vincentfretin
Copy link
Contributor Author

Thanks for merging, I'll fix the two remaining todos #662 asap.

@vincentfretin vincentfretin deleted the code-cleanup-plus-react-update branch December 9, 2022 08:31
vincentfretin added a commit to vincentfretin/aframe-inspector that referenced this pull request Dec 24, 2022
vincentfretin added a commit to vincentfretin/aframe-inspector that referenced this pull request Dec 24, 2022
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.

Update all the dependencies, mainly webpack and react
2 participants