-
Notifications
You must be signed in to change notification settings - Fork 7
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
test(image-utils): replace jest
with vitest
#5711
Conversation
import { int } from './types'; | ||
import { RGBA_CHANNEL_COUNT } from '.'; | ||
import { arbitraryImageData, arbitraryRect } from '../test/arbitraries.mjs'; | ||
import { crop } from './crop.js'; |
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.
Why are the imports changed to have an extension? And why are the files changed to .mts
?
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 changed test-utils-vitest
to use type: module
in package.json
because the files would be transpiled to CommonJS and we'd get a require('vitest')
in the resulting build. Here is the file you get if you require('vitest')
:
// vitest/index.cjs
throw new Error(
'Vitest cannot be imported in a CommonJS module using require(). Please use "import" instead.'
+ '\n\nIf you are using "import" in your source code, then it\'s possible it was bundled into require() automatically by your bundler. '
+ 'In that case, do not bundle CommonJS output since it will never work with Vitest, or use dynamic import() which is available in all CommonJS modules.',
)
So now test-utils-vitest/build/index.js
is ESM. If you then depend on test-utils-vitest
and try to import it in a CJS test file, you get this:
Error [ERR_REQUIRE_ESM]: require() of ES Module /media/parallels/WorkingFiles/code/vxsuite/libs/test-utils-vitest/build/index.js not supported.
Instead change the require of index.js in null to a dynamic import() which is available in all CommonJS modules.
Since I don't want to change the packages themselves to be ESM yet, I change the test files to be treated as ESM by renaming them to .mts
. This makes everything work out. Once we're using ESM, though, we need to import relative files including their target extension. So we add the .js
extension to the imports from the .mts
files.
Closes #5710