Skip to content
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

Running Jest doesn't find 'vanilla-jsoneditor' module #334

Closed
Rob-Negrete opened this issue Nov 2, 2023 · 21 comments
Closed

Running Jest doesn't find 'vanilla-jsoneditor' module #334

Rob-Negrete opened this issue Nov 2, 2023 · 21 comments

Comments

@Rob-Negrete
Copy link

Hi @josdejong -
I'm moving from the old jsoneditor to vanilla-jsoneditor. It was pretty easy, and the component works on runtime as a charm.

However, there's some weird issue when running test cases with jest.

My implementation is basically the same as JSONEditorReact.tsx. Then, when I run tests, this error is thrown:

 ● Test suite failed to run

    Cannot find module 'vanilla-jsoneditor' from 'JsonEditorReact.tsx'

    Require stack:
     JsonEditorReact.tsx

      at Resolver._throwModNotFoundError (../../../node_modules/jest-resolve/build/resolver.js:427:11)

I've installed vanilla-jsoneditor": "^0.18.11 and included as part of my dependencies.

"dependencies": {
    "react": "^16.14.0",
     ....
    "vanilla-jsoneditor": "^0.18.11"
  },

Accordingly, to what I've read, this can be related to jest-resolver not being able to resolve the types / mapping.

Am I missing something?

Thanks in advance.

Rob-Negrete added a commit to Rob-Negrete/svelte-jsoneditor that referenced this issue Nov 2, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
As described on josdejong#334

This attempts to make vanilla-jsoneditor module available, specially when working in monorepos.
@josdejong
Copy link
Owner

Thanks for reporting and working on a fix in #335.

How can we reproduce this issue exactly?

Do you have any pointers of other projects having similar issues with Jest? I would like to better understand what's going on.

@Rob-Negrete
Copy link
Author

Hi @josdejong,

it’s a monorepo project having several modules. I’ve added several libs recently, however this is the only one lib having the issue.

It is like if the module wasn’t being detected or something at least at testing time.

@josdejong
Copy link
Owner

It is like if the module wasn’t being detected or something at least at testing time.

Indeed, and the question is why. How can I reproduce this issue?

@Rob-Negrete
Copy link
Author

I tried to reproduce it on a separated project (standalone without monorepo) but worked fine. So, I'll to set up a tiny monorepo project and try to reproduce the error on it.
If I had success, I'll share it to you.

@josdejong
Copy link
Owner

Thanks Rob!

@Rob-Negrete
Copy link
Author

I'd sent you an invitation to my repo which you can use it to reproduce the error.

@josdejong
Copy link
Owner

Thanks for setting up the demo, I can reproduce the issue in your setup. It's quite a complex setup though 😅 .

I've done a bit of digging. The root cause seems to be that Jest does not have full ESM support (it is still marked experimental).

  1. Your solution to add a require field (CommonJS) in the package.json that points to the ESM file indeed works, see https://stackoverflow.com/a/75595580/1262753. It feels odd though to do that, I'm not sure whether other tools will get confused because of that, effectively breaking more than we fix with it.
  2. Various discussions suggest adding certain config for jest and ts-jest:
  3. To verify whether Jest's limited ESM support is indeed the cause, it may help to see whether other ESM only libraries give the same issue. If that is the case, we know that it isn't a vanilla-jsoneditor related issue. And if they do not give the same issue, we can have a look at their package.json to see what's the difference.

I haven't found a solution yet except for the require field hack. I have to say, I've intensively used a lot of different test libraries in the past, and most of them work great, only Karma and Jest where always giving me troubles. Jest has difficulties with TS and ESM and everything, therefore I moved away from it. I'm mostly using vitest nowadays, works like a charm.

I think there are three options to solve this issue:

  1. File a bug with Jest about this issue with an ESM only library.
  2. vanilla-jsoneditor is currently ESM only. Adding CommonJS support would solve the issue with Jest
  3. In your project, switch to an other test framework that works fine with ESM libraries.

@Rob-Negrete
Copy link
Author

Hi @josdejong, thanks for following this up :)

Can you tell me more about option 2, please?

Adding CommonJS support would solve the issue with Jest

@josdejong
Copy link
Owner

If vanilla-jsoneditor would contain a CommonJS version of the library (alongside ESM), we can let this require in the package.json point to the actual CommonJS version of the code.

However, this does not have my preference. in general I would love the JS ecosystem to move away from CommonJS and adopt ESM, not the other way around. The current hybrid situation is a big pain. That's why so far I haven't provided a CommonJS version. See: #196

@Rob-Negrete
Copy link
Author

Got it.

I'll try these configs you mentioned:

Various discussions suggest adding certain config for jest and ts-jest:
  + Use a --experimental-vm-modules flag, see https://jestjs.io/docs/ecmascript-modules.
  + Configure the useESM option of ts-node, see https://github.com/jestjs/jest/issues/13739#issuecomment-1655607116 and https://kulshekhar.github.io/ts-jest/docs/guides/esm-support/.
  + Configure a ts-jest/presets/default-esm preset, see
https://github.com/jestjs/jest/issues/13739#issuecomment-1655663846

And let it know if any of them works.

Otherwise, I might go back to jsoneditor 😞.

@josdejong
Copy link
Owner

josdejong commented Nov 15, 2023

As a workaround, you could also create a script that copies node_modules/vanilla-jsoneditor/* into some folder /src/assets/vanilla-jsonedtor, and then use the files from there: import { JSONEditor } from './src/assets/vanilla-jsoneditor/index.js'. It's ugly but it should work.

@Rob-Negrete
Copy link
Author

Pretty odd, yeh. But, might help for a while.

Thanks.

@Rob-Negrete
Copy link
Author

Sadly, none of them worked :(

@Rob-Negrete
Copy link
Author

Compared against @tinyhttp/app which it only has this:

"exports": "./dist/index.js",

instead of something like this:

"exports": {
    ".": {
      "types": "./index.d.ts",
      "module": "./index.js"
    },
    "./CHANGELOG.md": "./CHANGELOG.md",
    "./index.d.ts": "./index.d.ts",
    "./index.js": "./index.js",
    "./index.js.map": "./index.js.map",
    "./LICENSE.md": "./LICENSE.md",
    "./README.md": "./README.md",
    "./SECURITY.md": "./SECURITY.md",
    "./themes/jse-theme-dark.css": "./themes/jse-theme-dark.css",
    "./themes/jse-theme-default.css": "./themes/jse-theme-default.css",
    "./package.json": "./package.json"
  },

By doing this, it seems to work.

What do you think about it?

@josdejong
Copy link
Owner

Ahh, that is a good idea! We have to make sure that TypeScript can still find the types (should be the case because of the types in the root of package.json, but we have to check that.

@Rob-Negrete
Copy link
Author

Awesome! I tested it on my setup and it worked. Feel free to use it for your tests.

@josdejong
Copy link
Owner

😎 I'll do some testing next week, this looks promising.

@josdejong
Copy link
Owner

This issue should be fixed now in v0.19.0, can you give that a try @Rob-Negrete ?

@Rob-Negrete
Copy link
Author

Sure, I'll try it today and let you know ;)

@Rob-Negrete
Copy link
Author

Yei! It worked :)

@josdejong
Copy link
Owner

😎 Awesome!

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

No branches or pull requests

2 participants