Have locked in versions for dependencies

[BioPhoton]

Having inter-package dependencies like "@code-pushup/models": "*" in our package.jsons is a problem, they should all be locked into the same version. Otherwise only direct dependencies like cli get updated and indirect dependencies stay behind.

Todo:

• set fixed version for all packages
• Avoid “*” versions with eslint or a plugin

Research:

• would it help to replace jscutlery/semver with new nx release?

[getlarge]

I am willing to take on this task if it helps.

[getlarge]

A generator has been created to ease migration to nx release.
I should probably start with this.

[getlarge]

@BioPhoton status update:
I just opened a draft PR with ongoing progress and need some help.
To use some needed features from the nx release CLI, I upgraded to nx 17.3.2 (the last 17.x), which required upgrading vite (^5.0.0) and vitest (^1.0.0), and unfortunately, the migration to vitest is not straightforward.

• When running the unit-test target for core the test run appears to be stuck until timed out, with no error displayed even when using the --verbose flag.
• When debugging vitest run by the Nx executor, i could log the following error emitted by the vitest worker:

{
          "stack": "Error: [vitest-worker]: Timeout calling \"fetch\" with \"[\"/Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts\",\"ssr\"]\"\n    at Object.onTimeoutError (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/rpc.joBhAkyK.js:61:15)\n    at Timeout._onTimeout (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/index.8bPxjt7g.js:39:41)\n    at listOnTimeout (node:internal/timers:569:17)\n    at processTimers (node:internal/timers:512:7)",
          "message": "[vitest-worker]: Timeout calling \"fetch\" with \"[\"/Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts\",\"ssr\"]\"",
          "stackStr": "Error: [vitest-worker]: Timeout calling \"fetch\" with \"[\"/Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts\",\"ssr\"]\"\n    at Object.onTimeoutError (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/rpc.joBhAkyK.js:61:15)\n    at Timeout._onTimeout (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/index.8bPxjt7g.js:39:41)\n    at listOnTimeout (node:internal/timers:569:17)\n    at processTimers (node:internal/timers:512:7)",
          "nameStr": "Error",
          "expected": "undefined",
          "actual": "undefined",
          "constructor": "Function<Error>",
          "name": "Error",
          "toString": "Function<toString>"
        }

All test suites result in the same error.

The cherry on top: the process is left hanging and never exits appropriately.

Since that is my first experience with vite / vitest, I would appreciate some input to progress.

[getlarge]

After reading this vitest issue it seemed like fetch implementation in Node.JS might also be responsible for the hanging process, check this undici issue.

So I tried to rerun the tests on a single thread. Result: the process is not left hanging anymore.

export default defineConfig({
  cacheDir: '../../node_modules/.vite/core',
  plugins: [
    nxViteTsPaths({
      debug: true,
    }),
  ],
  test: {
    name: 'core',
    globals: true,
    cache: {
      dir: '../../node_modules/.vitest/core',
    },
    pool: 'threads',
    poolOptions: {
      threads: {
        singleThread: true,
      },
    },
    coverage: {
      reporter: ['lcov'],
    },
    environment: 'node',
    include: ['src/**/*.unit.test.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'],
    globalSetup: ['../../global-setup.ts'],
    setupFiles: [
      '../../testing-utils/src/lib/setup/fs.mock.ts',
      '../../testing-utils/src/lib/setup/git.mock.ts',
      '../../testing-utils/src/lib/setup/console.mock.ts',
      '../../testing-utils/src/lib/setup/reset.mocks.ts',
      '../../testing-utils/src/lib/setup/portal-client.mock.ts',
    ],
    workspace: './vitest.workspace.ts',
  },
});

And the test error is now more meaningful:

 {
          stack: 'Error: Failed to load url @code-pushup/testing-utils (resolved id: @code-pushup/testing-utils) in /Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts. Does the file exist?\n' +
            '    at loadAndTransform (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite/dist/node/chunks/dep-stQc5rCc.js:53572:21)\n' +
            '    at async ViteNodeServer._transformRequest (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite-node/dist/server.mjs:413:16)\n' +
            '    at async ViteNodeServer._fetchModule (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite-node/dist/server.mjs:379:17)\n' +
            '    at async MessagePort.<anonymous> (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/index.8bPxjt7g.js:65:20)',
          message: 'Failed to load url @code-pushup/testing-utils (resolved id: @code-pushup/testing-utils) in /Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts. Does the file exist?',
          stackStr: 'Error: Failed to load url @code-pushup/testing-utils (resolved id: @code-pushup/testing-utils) in /Users/edouard/Dev/code-pushup/cli/packages/core/src/lib/implementation/execute-plugin.unit.test.ts. Does the file exist?\n' +
            '    at loadAndTransform (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite/dist/node/chunks/dep-stQc5rCc.js:53572:21)\n' +
            '    at async ViteNodeServer._transformRequest (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite-node/dist/server.mjs:413:16)\n' +
            '    at async ViteNodeServer._fetchModule (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vite-node/dist/server.mjs:379:17)\n' +
            '    at async MessagePort.<anonymous> (file:///Users/edouard/Dev/code-pushup/cli/node_modules/vitest/dist/vendor/index.8bPxjt7g.js:65:20)',
          nameStr: 'Error',
          expected: 'undefined',
          actual: 'undefined',
          constructor: 'Function<Error>',
          name: 'Error',
          toString: 'Function<toString>'
        }

[getlarge]

A vite configuration that seems to work

/// <reference types="vitest" />
import { nxViteTsPaths } from '@nx/vite/plugins/nx-tsconfig-paths.plugin';
import { URL } from 'node:url';
import { defineConfig } from 'vite';

export default defineConfig({
  cacheDir: '../../node_modules/.vite/core',
  plugins: [
    nxViteTsPaths({
      debug: true,
    }),
  ],
  test: {
    name: 'core',
    globals: true,
    cache: {
      dir: '../../node_modules/.vitest/core',
    },
    alias: [
      {
        find: '@code-pushup/testing-utils',
        replacement: new URL('../../testing-utils/src', import.meta.url)
          .pathname,
      },
      {
        find: '@code-pushup/models',
        replacement: new URL('../models/src', import.meta.url).pathname,
      },
      {
        find: '@code-pushup/utils',
        replacement: new URL('../utils/src', import.meta.url).pathname,
      },
    ],
    pool: 'threads',
    poolOptions: {
      threads: {
        singleThread: true,
      },
    },
    coverage: {
      reporter: ['lcov'],
      enabled: false,
    },
    environment: 'node',
    include: ['src/**/*.unit.test.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'],
    globalSetup: ['../../global-setup.ts'],
    setupFiles: [
      '../../testing-utils/src/lib/setup/fs.mock.ts',
      '../../testing-utils/src/lib/setup/git.mock.ts',
      '../../testing-utils/src/lib/setup/console.mock.ts',
      '../../testing-utils/src/lib/setup/reset.mocks.ts',
      '../../testing-utils/src/lib/setup/portal-client.mock.ts',
    ],
    workspace: './vitest.workspace.ts',
  },
});

I would expect nxViteTsPaths to resolve those TS config paths...

[getlarge]

Still unable to make TS config path resolution working with Vitest, so i have been digging into issues and articles that seems related:

• Last migration update wroke the import via ts (problem with vite.config.ts and new nxViteTsPaths method)  nrwl/nx#18492
• Vite can't import from index.tsx after moving to nxViteTsPaths nrwl/nx#19349
• Conflicts with package-based monorepo and tsconfig paths aliases (Parsing issue with NxViteTsPaths) + solutions and fixes nrwl/nx#21030
• https://www.wolff.fun/path-mapping-typescript-vite/

When reading this particular comment I thought that Vitest could be confused that the scope @code-pushup is used by some actual (external) packages and would try to resolve the local packages path (such as @code-pushup/models, @code-pushup/testing-utils, …) from the node_modules, leading to the Failed to load url @code-pushup/testing-utils error message.

If anyone has a clue what could go wrong, please share your knowledge :D
Otherwise, i will keep on reading and will venture into debugging how files are resolved by Vite.

[getlarge]

In the latest episode...
@BioPhoton joined to participate in a debug session, and we noticed the following:

• when running npx vitest --config=vite.config.unit.ts from the package directory, TS config paths are resolved thanks to nxViteTsPaths
• when patching @nx/vite:test executor to change dir before calling startVitest and reflect the path in the vitest options, vite resolve the TS paths correctly.

This is what the (ugly) patch looks like in node_modules/@nx/vite/src/executors/test/vitest.impl.js

// ...
+    resolvedOptions.root = '.';
+    resolvedOptions.config = _path.basename(resolvedOptions.configFile);
+    process.chdir(_path.join(process.cwd(), projectRoot));
    const ctx = await startVitest((_resolvedOptions_mode = resolvedOptions['mode']) != null ? _resolvedOptions_mode : 'test', cliFilters, resolvedOptions);
// ...

I still don't understand:

• how this root/config path interferes with TS config paths resolution
• which project should be fixed (nx/vite? vitest? code-pushup?)

[getlarge]

I just created an issue in Nx about that...

[matejchalk]

@getlarge First of all, thanks a lot for all your effort ❤️ This Nx/Vitest update turned out to be a huge pain, but using all your fixes and comments saved me a lot of extra time and frustration 🙏

I managed to eventually put together an Nx 17.3.2 update with a fully passing CI. There are a couple of failing integration tests left which in the end I decided to skip for now (ESLint plugin runner + Git helpers), so that we get something working and unblock other work.

Overall, I included most of the fixes you came up with:

• setting everything to single-thread, otherwise it would hang/timeout 👍
  • the one downside here is it turns out the plugin-eslint integration tests only work with multiple threads 😬 - digged into them for quite some time, but in the end decided to leave it for another day 🥱
• global setup paths are now indeed relative to project root 👍
• the nxViteTsPaths() plugin doesn't seem to do anything for Vitest anymore (also tried vite-tsconfig-paths to no avail - in general I got the impression none of the plugins were being called) - but setting the test.alias worked 👍
  • to make it easier to maintain I created a helper function in tools/vitest-tsconfig-path-aliases.ts so that we can just do alias: tsconfigPathAliases() in all vite configs - the function generates the aliases from paths in tsconfig.base.json
  • I dropped nxViteTsPaths() from plugins completely

I then ended up having to do some additional fixes, the main ones being:

• updated snapshots according to new Vitest formatting ("s are not escaped anymore)
• teardown functions from global setup files weren't being called anymore because Nx was suddenly running vitest in watch mode - fixed by adding "watch": false to executor defaults in nx.json
• the coverage configuration for examples/react-todos-app had to be tweaked post-update, we run E2E tests against it for our coverage-plugin
• the reportsDirectory in project.json only was being ignored, had to copy it to vitest config for all projects
• running tests with --coverage would crash with TypeError: Cannot read properties of undefined (reading 'push') after update, seems related to --coverage.100 crashes with TypeError vitest-dev/vitest#4307 - fixed by using --coverage.enabled as suggested in issue

I have all these changes in the following PR (CI passes 😌):

• build: migrate Nx to 17.3.2 and fix test problems #532

I think it's best to merge a working update first before proceding with nx release work, so that the scope doesn't get out of hand 🙏 But I'm also happy to merge a PR you author instead (should also be reviewed by @Tlacenka as she's our QA), since you did the heavy lifting it would be nice to have the contributions under your name 🙂 Let me know your thoughts.

And once again, huge thanks for digging into this mess ⭐

cc @BioPhoton @Tlacenka

[getlarge]

@matejchalk I am glad this investigation could make your life easier and you are done with the migration.

I think it's best to merge a working update first before proceding with nx release work, so that the scope doesn't get out of hand 🙏 But I'm also happy to merge a PR you author instead (should also be reviewed by @Tlacenka as she's our QA), since you did the heavy lifting it would be nice to have the contributions under your name 🙂 Let me know your thoughts.

Yes, solving this migration before going further with nx release is better. You can ping me once #532 is merged and i will continue #511 from the main branch.

[matejchalk]

@getlarge The update is merged and we fixed the remaining integration tests as well. So there should be no more blockers for working on nx release 🤞

[getlarge]

@matejchalk I'll try to put #511 back on the rails this week.