Skip to content

Conversation

@nadr0
Copy link
Contributor

@nadr0 nadr0 commented Nov 17, 2025

root causing

  • we use vitest
  • vitest probably global injects
  • tsc yells because we do not have vitest globally defined in the builder
  • we literally said "we have jest, and mocha!"
  • tsc passes
image

tsc passes because it thinks this is jest

image

tsc passes because it thinks this is jest

If you remove all jest related stuff, it will then think its mocha!

image

even after removing jest and mocha it still thinks it is jest and mocha and not vitest :(

issue

It looks like jest, mocha, and vitest are all configured in some way or imported and this is absurdly confusing.

implementation

vitest is probably globally importing describe. When you check your lsp or tsc command it says it is defined because we told tsconfig.json that we have mocha installed. So the LSP and tsc check were saying that all of our describe functions or test functions are mocha functions when it is not.

Similar to jest. Jest had configurations and packages installed. This makes it so confusing.

https://github.com/KittyCAD/modeling-app/pull/8952/files#diff-b55cdbef4907b7045f32cc5360d48d262cca5f94062e353089f189f4460039e0L21

Since vitest is most likely globally injecting all the functions our project configuration said we were using jest and mocha so all the global imports that are not defined pass tsc but for the wrong test runner.

@vercel
Copy link

vercel bot commented Nov 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
modeling-app Ready Ready Preview Comment Nov 18, 2025 7:25pm

"plugins": [
"react-perf",
"css-modules",
"jest",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing jest configurations. We do not need jest.

if (!tronApp) {
fail()
}
if (!tronApp) throw new Error('tronApp is missing.')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not know how fail() was implemented before but it had to have been from jest or mocha global configurations.

I've oped to say throw an error if it is missing to crash the test.

"@types/diff": "^7.0.2",
"@types/hammerjs": "^2.0.46",
"@types/isomorphic-fetch": "^0.0.39",
"@types/jest": "^30.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing all jest and mocha related packages.

"vite/client",
"@types/wicg-file-system-access",
"node",
"mocha",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing mocha

@nadr0 nadr0 marked this pull request as ready for review November 18, 2025 16:42
@nadr0 nadr0 requested review from a team as code owners November 18, 2025 16:42
@nadr0 nadr0 changed the title chore: delete all test runners other than vitest chore: delete jest, delete mocha, only use vitest Nov 18, 2025
// as well as import your extension to test it
import * as vscode from 'vscode'

import { suite, test } from 'mocha'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is importing from mocha intentional here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes otherwise the tsc will fail.

We have tsc on all *.ts files in the entire repository. Since I globally removed mocha from the tsconfig.json types this rust folder that has its own javascript library with another package.json needs this import.

This package under rust uses mocha which is different from ZDS's package.json and unit test library.

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.

3 participants