streaming-video-technology-alliance / common-media-library

A common library for media playback in JavaScript
Other
43 stars 2 forks source link

Cannot import library in jest tests #98

Closed story75 closed 2 months ago

story75 commented 3 months ago

Describe the bug When importing @svta/common-media-library e.g. in a TypeScript file like this:

import { getId3Frames } from '@svta/common-media-library';

All files which run through jest e.g. via the test file directly or through other files trigger the following error

Must use import to load ES Module: /Users/a/git/project/node_modules/@svta/common-media-library/dist/cjs/index.js

To Reproduce Steps to reproduce the behavior:

  1. Clone https://github.com/story75/common-media-library-jest-issue
  2. Run npm install
  3. Run npm test
  4. See error

Expected behavior The module should work with common tooling like jest.

Screenshots

Platform (please complete the following information):

Additional context The culprit behind this is that jest will try to determine if the file is ESM or not and wrongly decide that all files are ESM, because the package.json defines "type": "module".

Especially with resolvers like @swc/jest you cannot include the package and test your files anymore. Unfortunately the "resolver" option of jest does not influence its "shouldTreatAsESM" check.

In this case the package uses exports and defines CJS and ESM so setting type=module is superfluous and breaks tools like jest.

Userland the only fix is not to use jest or patch the package.json to remove the "type" field e.g. in a prepare script.

littlespex commented 2 months ago

Setting "type": "module" is not superfluous in this case, its tells node that the repo's build and test scripts are ESM. Without additional changes, removing it would break CI and dev scripts.

littlespex commented 2 months ago

Fixed in 0.7.2

story75 commented 2 months ago

Hey opening this again, because the change was applied in the wrong package.json, the one of the workspace instead of the sub packages. I've also updated the example.

The @svta/common-media-library still defines "type": "module" and thus the issue is not solved.

I didn't mean to come of rude, since the statement was from a consumer standpoint, where the exports field already correctly resolves to either ESM or CJS, but unfortunately tools like Jest for whatever reason do the import correctly but then also check the type field and end up getting it wrong regardless.

For the workspace the type module can stay, it's just that the published package.json when pulling the package ends up causing errors when type:module is present.

So from the structure of the repo only the packages under /lib are affected which is just "@svta/common-media-library".

littlespex commented 2 months ago

Good catch. The issue now is that removing "type": "module" from the package json in lib breaks other projects that use CML in production. This will require further investigation.

littlespex commented 2 months ago

Looks like the main issue was ts-node. This issue should be fixed in 0.7.3. Let me know if you run into any issues.