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

Bump deps #92

Merged
merged 18 commits into from
Nov 26, 2022
Merged

Bump deps #92

merged 18 commits into from
Nov 26, 2022

Conversation

NoahBres
Copy link
Contributor

@NoahBres NoahBres commented Sep 25, 2022

Significant package changes:

  • Bump node version in gradle build
    • FtcDashboard/build.gradle
    • 12.13.0 -> 18.12.1
    • 18.12.1 is the latest LTS version
      • Went through the changelog for versions 12 -> 18. There are no breaking changes.
    • Node version 16+ is required for the build to work on m1 macbooks
  • vite: 2 -> 3
    • https://vitejs.dev/blog/announcing-vite3.html
    • Cold start improvements
    • Architecture changes for SSR. Irrelevant
    • @vitejs/plugin-react-refresh uninstalled, replaced by @vitejs/plugin-react
    • tsconfig compiler option targeting ESNext
      • ES5 option deprecated
    • react-scripts package removed. Relic from create-react-app
  • react: 17 -> 18
  • redux: 4.0 -> 4.2
  • react-redux: 7 -> 8
  • redux-thunk: 2.3 -> 2.4
    • Internal TypeScript codebase rewrite
  • tailwindcss: 2 -> 3
    • https://tailwindcss.com/blog/tailwindcss-v3
    • JIT by default. Removal of @tailwindcss/jit package
    • Colored box shadows. No need for custom extension in tailwind.config.js. Removed.
    • Variants on by default thanks to jit
    • Colors enabled by default thanks to jit

Misc

  • Pre-commit hooks removed
    • Although ideal to ensure consistency, this introduces unnecessary friction during development
      • Instead, instructions for ESLint and Prettier autoformatting will be included and encouraged. Checks should be done prior merge rather than per-commit.
      • I am sure literally zero people used these anyways as git commit hooks need to be explicitly installed upon clone
  • Add npm run dev, equivalent to current npm run start
    • npm run dev seems to have overtaken npm run start as the standard in terms of default ecosystem documentation assumptions. Just a small convenience change
  • Tailwind CSS:
    • Every color in the palette is included by default now. Thus, colors are no longer overriden
      • yellow -> amber
    • Colored shadows were implemented. Custom shadows were removed
    • A number of classes were moved around by an updated auto-formatter
  • Fixing bug where state is entirely cleared rather than appended to in OpModeView (introduced in Address getGamepads changes #72)
  • Type changes to Redux Thunk. Ignored in socketMiddleware.ts
  • React 18 createRoot

@NoahBres NoahBres marked this pull request as ready for review November 21, 2022 20:35
@NoahBres
Copy link
Contributor Author

NoahBres commented Nov 21, 2022

Verified on running opmode ✅

@NoahBres NoahBres mentioned this pull request Nov 23, 2022
@rbrott
Copy link
Member

rbrott commented Nov 24, 2022

"Bump deps" definitely undersold this. Thanks for some much-needed maintenance!

Pre-commit hooks removed

I use them 😄 Can you elaborate on the development friction? Does it just encourage fewer, bigger commits? I'd generally rather handle formatting this way than set up GH actions or something.

Fixing bug where state is entirely cleared rather than appended to in OpModeView (introduced in Address getGamepads changes #72)

I didn't even catch it on my second review. Silly setState() semantics.

@NoahBres
Copy link
Contributor Author

I use them 😄 Can you elaborate on the development friction? Does it just encourage fewer, bigger commits? I'd generally rather handle formatting this way than set up GH actions or something.

This is entirely irrelevant but regarding previous work in a professional setting, they just proved to be an annoyance as the hooks are generally very slow and you were discouraged from making frequent commits. Commits failing for linting errors felt incredibly unnecessary since you were not really concerned until merging into the main branch. I've had similar discussions with other teams.
However, this is completely irrelevant to dash's development given that so few people work on it—and so infrequently. Also, my laptop is fast enough where I don't notice how slow hooks used to be—unsure if that carries for average users. I just wasn't aware that anyone used it since they are not automatically installed and recent PR's have not complied. I was working off of a new device and had forgotten that the pre-commit hooks existed.
I think if you find this useful, I will definitely re-include them. I am going to add some documentation eventually so it is encouraged to set them up.

@rbrott rbrott merged commit 4f7d0a6 into acmerobotics:master Nov 26, 2022
@NoahBres NoahBres deleted the bump-deps branch November 26, 2022 17:32
j5155 pushed a commit to jdhs-ftc/ftc-dashboard-record that referenced this pull request Jul 22, 2024
* Bump deps

* Remove redux types

* Remove CRA relic

* Create index.d.ts and add svgr types

* Fix type errors and state overwriting in OpModeView

State being overwritten rather than appended to in this.setState() calls

* Replace node's process env with vite's

https://vitejs.dev/guide/env-and-mode.html#modes

* Switch from mode string to DEV boolean

* Ignore socketMiddleware type errors

* Switch to Tailwind 3 colored shadows

* Bump dep

* Remove package-lock.json

* React 18 createRoot

* Bump gradle node version to latest LTS

* Remove redundant React import

* New yarn.lock

* Reinstate pre-commit hooks
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