-
Notifications
You must be signed in to change notification settings - Fork 16.3k
build(ui): upgrade Vite to v7 & Vitest to v3; require Node >=22 #55593
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
build(ui): upgrade Vite to v7 & Vitest to v3; require Node >=22 #55593
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
ashb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need one of @pierrejeambrun @bbovenzi to look at this too.
I wonder if this has any run-time implications for UI React plugins now I think about it 🤔
| environment: "happy-dom", | ||
| globals: true, | ||
| mockReset: true, | ||
| passWithNoTests: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this flag mean/do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this flag mean/do?
It is a vitest only option that makes the run succeed when no test files are found otherwise vitest exits with code 1. I added it because the React plugin template and Edge plugin currently have no tests, so CI would fail.
This key is only read by Vitest and Vite ignores test so there is no runtime/build impact on UI React plugins.
If you prefer to keep vite.config.ts build only, I can move it to the script instead: vitest run --passWithNoTests, or drop it and add a tiny placeholder test.
|
@KatalKavya96 thanks for bumping the dependencies! I also wanted to get started with this. Unfortunately atm the compiled assets for the edge plugin are checked in (and not dynamically compiled) therefore static checks fail. (I'd push the fix to your branch but unfortunately I am not able to, Github does not allow me :-( ) |
Thanks for the clear pointers! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this and changes look good (after fixing static checks locally). Plugins still working good. Clicked a bit around in UI and no issue "seen" - but was just doing basic tests of critical UI elements.
I would like a second pair of expert review by @bbovenzi or @pierrejeambrun - but from my side "green light" (Can be merged of course only after fixing static checks...)
dfb6e3e to
e1bc7c0
Compare
|
@jscheffl Thanks! I fixed the eslint issues and prek run compile-edge-assets -a . I messed up rebase/branching and the PR branch diverged. Kindly test the latest commit and suggest any changes and cleanups |
Thanks! I think this looks good still. No need to worry, the last commit made all good. As (I don't know why) can not push to your branch (even I am maintainer) the other alternative would be taking your commits into an own branch and opening another PR... but would leave the credits for the fixes to you :-D |
|
You can also cherry-pick the merge from PR # #55617 ... if that is easier. |
d3ab321 to
d4cdee9
Compare
|
@jscheffl I tried syncing my fork , merging conflicts , rebasing and commiting the latest but somehow it has closed this PR, Please suggest me next course of action as i am kind of stuck here. Should i create a new PR? |
Sometimes can happen that you mess-up your rebase/merge. Seems you re-based and I assume commits are "gone". If you are uncertain hard to know what you did locally. Have you tried Otherwise I can offer taking over to my PR or you open another PR... (Would leave the credit for PR to you but I see your commits are also on my PR so post-merge commits "from you" are also in the codebase :-D |
Summary
Upgrade frontend build/test tooling to current majors:
Vite → v7 and Vitest → v3 across UI packages.
Switch coverage to @vitest/coverage-v8 where applicable.
Set "engines": { "node": ">=22" } for packages that build UI assets (per maintainer guidance that Node 22 is acceptable for dev/asset builds).
Packages updated
airflow-core/src/airflow/ui
airflow-core/src/airflow/api_fastapi/auth/managers/simple/ui
dev/react-plugin-tools/react_plugin_template
providers/edge3/src/airflow/providers/edge3/plugins/www
Notes
No runtime feature changes; these are dev/build-time upgrades only.
Vite configs for the plugin template and Edge provider were minimally touched to remain compatible with Vite 7 and preserve existing behavior.
No built artifacts are committed; only package.json and subpackage pnpm-lock.yaml updates.
Verification (local)
pnpm install
pnpm build (Vite 7)
pnpm test -- --run (Vitest 3, with @vitest/coverage-v8 where defined)
All builds succeeded and tests passed locally. Any transient ECONNREFUSED :3000 console noise observed during tests does not affect pass status.
Follow-ups (if desired)
closes: #55562