josdejong / svelte-jsoneditor

A web-based tool to view, edit, format, repair, query, transform, and validate JSON
https://jsoneditoronline.org
Other
818 stars 108 forks source link

"Failed to resolve import from ajv" error #271

Closed Metehan-Altuntekin closed 10 months ago

Metehan-Altuntekin commented 1 year ago

I am getting this error when I try to use it in a Svelte workspace in a monorepo:

"Failed to resolve import "ajv/dist/runtime/validation_error" from "node_modules.vite\deps\svelte-jsoneditor.js?v=dab92afd". Does the file exist?"

image

When I check node_modules, ajv appears to be not installed: image

When I add ajv as a dependency to the workspace, it appears in the node_modules error goes away.


System: Windows 10 Pro Node: 18.13.0 PNPM: 8.1.1

Project: https://github.com/cozemble/monorepo Workspace: https://github.com/cozemble/monorepo/tree/main/frontend/datatable

josdejong commented 1 year ago

The package.json of the npm library svelte-jsoneditor does contain ajv in the dependencies. I do not understand why any client would not install the dependency? Can you provide a minimal example demonstrating the issue?

See package.json: https://unpkg.com/svelte-jsoneditor@0.17.3/package.json

Metehan-Altuntekin commented 1 year ago

Yeah, that is really weird isn't it? I think it's related to pnpm more than svelte-jsoneditor itself. Even when it is not a monorepo, ajv is not installed properly when I use pnpm but npm or yarn installs just right.

Example of installing with pnpm in a non monorepo poject:

image

Example of installing with npm in a non monorepo project:

image

Metehan-Altuntekin commented 1 year ago

Found more details, apparently pnpm installs package dependencies into node_modules/.pnpm/ directory like so:

image

The non-monorepo pnpm installed repository works just fine. But the error occurs in the monorepo one even though it installs ajv to that directory too:

image

But in a monorepo, ajv is installed in the node_modules/.pnpm/ directory of the repo root. Not in the workspace's node_modules/.pnpm of the nested workspace. This behaviour is not exceptional for ajv or svelte-jsoneditor though, it appears to be the way of doing things for pnpm.

josdejong commented 1 year ago

Maybe this topic is relevant: https://stackoverflow.com/questions/70597494/pnpm-does-not-resolve-dependencies

Metehan-Altuntekin commented 1 year ago

Thanks @josdejong, it helped with the error but the trade-off is that it allows to import non declared dependencies and that is not ideal. I found that enabling that only for ajv with public-hoist-pattern[]=ajv also works and it's less dangerous. Still, it will not prevent importing ajv in workspaces that doesn't have it as a dependency so not finding it ideal.

I think the issue may be more closely related to Vite, since this error occurs within a Vite Dependency Pre-Bundling.

I have checked the same file at 2 different cases and there are differences in way it imports ajv

When I install with pnpm i:

image

When I install with pnpm i --shamefully-hoist=true:

image

Isn't it interesting that about 300 of results just disappear?

josdejong commented 1 year ago

I'm not sure how I can help. Is there anything in svelte-jsoneditor that we can change to improve this? Or do we have to report something at ajv or pnpm or vite?

Metehan-Altuntekin commented 1 year ago

I'm not sure either. My best guess is it's Vite but I can't speak for sure without examining the code

josdejong commented 1 year ago

I think the next steps are:

Anyone able to help figuring this out?

Metehan-Altuntekin commented 1 year ago

I think I can create a demo repo in some of my free time. I'll add it to my Todo

josdejong commented 1 year ago

Thanks!

Metehan-Altuntekin commented 10 months ago

Sorry for the delay. I have been trying to avoid little things as much as possible for a while.

I have just created the demo repo: (https://github.com/Metehan-Altuntekin/svelte-jsoneditor-pnpm-monorepo-issue-demo)

With the latest version of everything, the issue seems to be fixed.

josdejong commented 10 months ago

That is good news, thanks for the feedback 🎉