feat(angular): add vitest option to angular#27311
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 80e346e. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 3 targetsSent with 💌 from NxCloud. |
Coly010
left a comment
There was a problem hiding this comment.
Looks great! I need to test it out properly, but other than the comments I've added, I don't see anything untoward.
Could you add some e2e tests that cover using Vitest as a selected option so that we can ensure behaviour continues to work going forward?
Of course! By the way, I also wanted to add an e2e to test the watch mode which is generally a common issue. As a matter of fact, the watch was detecting file changes and running the tests without rebuilding the TS. For some reason, I had to upgrade the angular plugin to 1.7.0 to make it work. |
d5136c4 to
03bdf3d
Compare
| * @param path The path relative to the workspace root to start searching from. | ||
| * @param fileName The name of the file to search for. | ||
| */ | ||
| export function findFileInClosestParentFolder( |
There was a problem hiding this comment.
It looks like this function is used for the purpose of detecting whether Node will treat a .js or .ts file as ESM or not (due to "type": "module" in package.json).
My suggestion is to explicitly use .mts (or .mjs, .cjs, .cts) file extensions so the format is controlled rather than let Node interpret it. In that case, we don't need this function in devkit.
There was a problem hiding this comment.
Exactly! I thought that using .ts when possible would be less confusing to users and machines.
For instance, Playwright VSCode Plugin does not detect playwright.config.mts files. So there might be some plugins/tools that do not detect vite.config.mts.
Other than that, if you are fine with generating vite.config.mts all the time, we can get rid of this function and I'll be happy to fix this.
There was a problem hiding this comment.
Let's generate vite.config.mts all the time and we can remove this function :)
|
Is there any news on an ETA when this awesome new feature is gonna be available? |
70db017 to
8262f3c
Compare
|
Deployment failed with the following error: |
c3db560 to
3571d5a
Compare
|
|
I'd much rather pay the maintenance tax for updating minors more frequently given we support current Angular Major + 2 previous major versions with Nx. We can ensure better stability with all the versions we support this way. |
|
Oh s*** back to Nx again.... cool stuff @yjaaidi 🔥🔥🔥 |
|
This is going to be so 🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥🔥, Now we have to wait for serving/building to be under vite. |
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
vitestoption tounitTestRunnerfor Angular generators.create-nx-workspacebut I think we should provide this as an option in the future. Please let me know if you wantme to add this here or as a future feature.