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 dependencies && remove unused vars #704

Merged
merged 5 commits into from
Sep 29, 2023
Merged

Conversation

chinatsu
Copy link
Contributor

@chinatsu chinatsu commented Sep 28, 2023

pretty huge pr here, but i promise that all i've done is

  1. update dependencies (i used npx npm-check-updates --interactive --format group to check)
  2. fix backwards incompatible changes with the react-markdown dependency
  3. go through the compiler warnings and remove all instances of no-unused-vars, and some other eslint issues.
    • i have not verified that everything works as intended, but i don't think we'll need to do that either. we'll create new tasks if we see something bad as a result of this, maybe? :)
  4. circumvent a problem regarding source maps and create-react-app.
    • the solution is basically to ignore the warnings due to the unmaintained state of create-react-app, we cannot fix it properly.
  5. replace a couple of dependencies that have not been updated in many years.
    • there were replacement libraries which i could use to almost fully drop in without much changes.

i did set out to solve all of the npm i issues, but it's not possible. the dependencies that baseui pull in do not support react 18, so in order to fix those problems we'll have to toss out all of baseui.


so generally what i've learned is, if we want to use react 18:

  • baseui shouldn't be used
  • create-react-app shouldn't be used

both of these points involve some difficult choices and potentially lots of work.

Copy link
Contributor

@JeremiahUy JeremiahUy left a comment

Choose a reason for hiding this comment

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

lgtm, kjørte branchen lokalt og for å dobbel sjekk :)

@chinatsu chinatsu merged commit e6ce333 into master Sep 29, 2023
@chinatsu chinatsu deleted the update-dependencies branch September 29, 2023 08:11
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.

2 participants