react:app generator with --bundler=vite ignores --unitTestRunner

MNishimuraBonfire commented 1 year ago

Current Behavior

nx g @nx/react:app my-app --bundler=vite --unitTestRunner=jest installs vitest and configures vitest as the test runner. This is also true of nx g @nx/react:app my-app --bundler=vite --unitTestRunner=none

Expected Behavior

--unitTestRunner should be respected

GitHub Repo

https://github.com/nrwl/nx-examples

Steps to Reproduce

Run nx g @nx/react:app my-app --bundler=vite --unitTestRunner=jest
Observe vite.config.ts contains vitest config.

Nx Report

>  NX   Report complete - copy this into the issue template

   Node   : 18.17.1
   OS     : darwin-arm64
   npm    : 9.6.7

   nx                 : 16.8.0
   @nx/js             : 16.8.0
   @nx/jest           : 16.8.0
   @nx/linter         : 16.8.0
   @nx/workspace      : 16.8.0
   @nx/angular        : 16.8.0
   @nx/cypress        : 16.8.0
   @nx/devkit         : 16.8.0
   @nx/eslint-plugin  : 16.8.0
   @nx/react          : 16.8.0
   @nrwl/tao          : 16.8.0
   @nx/vite           : 16.8.0
   @nx/web            : 16.8.0
   @nx/webpack        : 16.8.0
   nx-cloud           : 16.3.0
   typescript         : 5.1.3
   ---------------------------------------
   Community plugins:
   @ngrx/component-store : 16.0.0
   @ngrx/effects         : 16.0.0
   @ngrx/entity          : 16.0.0
   @ngrx/router-store    : 16.0.0
   @ngrx/store           : 16.0.0
   @ngrx/store-devtools  : 16.0.0

Failure Logs

No response

Package Manager Version

npm 9.6.7

Operating System

[X] macOS
[ ] Linux
[ ] Windows
[ ] Other (Please specify)

Additional Information

Appears introduced by https://github.com/nrwl/nx/pull/13436, which comments on behaviour later changed by https://github.com/nrwl/nx/pull/14188.

As far as I can tell, removing the

  if (options.bundler === 'vite') {
    options.unitTestRunner = 'vitest';
  }

behaves sanely for the initial generator run, though I may be not understanding additional context about how test generators behave later. If the intent is that using Vite as a bundler must use Vitest, that should be documented in the generator.

mandarini commented 1 year ago

@MNishimuraBonfire yes, that is correct, when using vite then vitest is used as well, not jest. You are correct, I will add this in our docs. I will fix that none is respected though.

Let me know what you think about that, though! Thank you for pointing it out!

MNishimuraBonfire commented 1 year ago

Allowing none is probably good enough to limit my annoyance with it; I'm working a repo where we switched to Vite as a bundler (and switched to NX) over Webpack for build speed reasons, but the tests are Jest (and there are a lot of them, test speed in CI is a major concern, and I'd prefer to not intermingle test runners, though NX does obviously support that), so would loosely prefer that configuration be supported.

Given vitest 0.34.0 (re: https://github.com/vitest-dev/vitest/issues/579 ) seems to be improving performance concerns, it may be worth taking the time to see if switching over entirely makes sense again.

That said, as long as I can give instructions to generate a new project without installing vitest as a side effect, and having to explain how to rip out its configuration, it's an improvement.

mandarini commented 1 year ago

Thank for the feedback @MNishimuraBonfire ! Good to know!

github-actions[bot] commented 11 months ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

nrwl / nx