Skip to content

Removing dva dependency#848

Merged
angrave merged 25 commits into
stagingfrom
remove-dva-dependency
Jun 26, 2025
Merged

Removing dva dependency#848
angrave merged 25 commits into
stagingfrom
remove-dva-dependency

Conversation

@dsding2

@dsding2 dsding2 commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@dsding2 dsding2 marked this pull request as ready for review June 25, 2025 23:38
@dsding2 dsding2 changed the title Draft: Removing dva dependency Removing dva dependency Jun 25, 2025
@dsding2

dsding2 commented Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

Note: the dependency tree is pretty messy, so running yarn remove dva causes a bunch of cascading errors.

Also, azure-app-insights is almost certainly broken, since I'm pretty sure useRouter does not exist outside of dva, but I'm not sure if azure-app-insights is even still used at all.

Other than that, everything should be ready to be merged into staging.

@angrave

angrave commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator

Exciting moment to see dva removed. Re. app-insights: Microsoft's app-insights is interesting .. but it requres Azure credits and is a bit of a privacy concern. We haven't used it for many years and should be removed.

@angrave angrave self-requested a review June 26, 2025 17:16
@angrave angrave merged commit 6878480 into staging Jun 26, 2025
2 checks passed
@angrave

angrave commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator

FYI I see a bunch of dependency warnings with react hooks in the docker build, typically
"Either include it or remove the dependency array"
eg
#21 126.2 src/screens/Instructor/CourseSettings/components/Staffs/index.js
#21 126.2 Line 47:6: React Hook useEffect has a missing dependency: 'getInstructors'. Either include it or remove the dependency array react-hooks/exhaustive-deps

@angrave

angrave commented Jun 26, 2025

Copy link
Copy Markdown
Collaborator

I couldn't wait for the hourly cron job - I manually did docker compose pull, and up -d, on ct-dev.This new dva-free version is alive on ct-dev.ncsa! Excellent work.,

@dsding2

dsding2 commented Jun 26, 2025

Copy link
Copy Markdown
Contributor Author

The warnings are probably outside of the scope of this pull request, but we should go back and fix this issue, as well as the other massive pile of eslint warnings.

@dsding2 dsding2 deleted the remove-dva-dependency branch July 5, 2025 14:59
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