jsonnext / codemirror-json-schema

A JSONSchema enabled mode for codemirror 6, for json4 and json5, inspired by monaco-json
https://codemirror-json-schema.netlify.app
MIT License
57 stars 9 forks source link

Error message: can't find module #66

Closed honigbienlein closed 5 months ago

honigbienlein commented 9 months ago

Hey guys,

I have installed you package codemirror-json-schema as described in your manual and integraded in my code:

<CodeMirror
                value={jsonTemplate}
                theme={dracula}
                indentWithTab
                extensions={[
                    json(),
                    EditorView.lineWrapping,
                    bracketMatching(),
                    /* linter(jsonLinter), */
                    lintGutter(),
                    jsonSchema(schema),
                ]}
                onChange={onChange}
                height='100vh'
            />

As soon as I start my app, everything looks fine. But when I refresh the page, I always get the following error: Error: Cannot find module '/.../node_modules/codemirror-json-schema/dist/json-completion' imported from /.../node_modules/codemirror-json-schema/dist/index.js

I checked the node_modules and it looks fine...

Am I the only one having this issue?

acao commented 9 months ago

I can think of a few reasons this would happen - I will look into this soon!

honigbienlein commented 9 months ago

@acao Do you have any updates on this?

acao commented 9 months ago

I'm looking into it and I can't seem to reproduce the issue - what bundler are you using? What version? Can you create a reproduction on codesandbox or stackblitz?

honigbienlein commented 9 months ago

I am using nextjs 13.4.12 and turbopack. I will try to reproduce something on codesandbox...

acao commented 9 months ago

oh thank you that's helpful! i will try and re-create the bug now, don't worry about recreating the bug, it should be fairly easy. also can you give me your node version to be sure? are you using typescript and if so can you share tsconfig? i have a few more theories to try. i think we need to use full path .js imports and set moduleResolution to something more modern

(for reference, we're using vite, which might explain why I couldn't reproduce it)

acao commented 9 months ago

@honigbienlein I have a working example with next and turbo at #68 if that's helpful! it used the latest next-create-app.

however, I need to move this example to codesandbox, because this doesn't recreate the full module import scenario necessarily, though the relevant factors seem to be lined up. In this example, I point to the dist directory and the next bundler appears to work, and the published files in dist are the same as the ones published on npm! I will let you know if the codesandbox demo reveals anything else

acao commented 9 months ago

Here is a codesandbox example working with next and turborepo and the imported library, perhaps you can use this as a guide and see where the config diverges to re-create the bug?

https://codesandbox.io/p/sandbox/objective-pascal-v4dss6

noting that, in tsconfig.json for codesandox's next template, their default for moduleResolution is node. in my create-next-app example in the PR, the moduleResolution is bundler. and one possibility I want to eliminate is to see whether your moduleResolution setting is one of the newer options such as nodenext or node-16 which might cause import issues until I merge #63

honigbienlein commented 9 months ago

@acao thanks for analyzing. Here are some more information:

acao commented 9 months ago

I have a codesandbox where I try matching everything but @uiw/react-codemirror. Node version, tsconfig, next version, almost everything matching.

I have a feeling that this is the ts config you possibly are missing, and it is somehow automatically enabled for me: image

Why didn't I try @uiw/react-codemirror? The problem is, though this is a major difference in our implementations, the standard runtime import error shows that the issue is related to bundling or some environment issue that is impacting bundling or runtime file resolution.

to illustrate what i mean, I bet that if you comment out this line but leave the import, it will still throw the same error:

          /* linter(jsonLinter), */
                    lintGutter(),
                    /* jsonSchema(schema), */
                ]}

if the tsconfig fix listed in that message at the beginning of my reply doesn't work on a full rebuild (delete .next and run dev again), I will try @uiw/react-codemirror

On our end:

If this is an issue with file casing, then we need to use consistent lower case file names throughout the project. currently we are using mixed casing styles which is very confusing haha

honigbienlein commented 8 months ago

@acao Thank you for looking into it. I adjusted my tsconfig, deleted .next and tried it again, unfortunately I am still facing the same issue

henrikhertler commented 8 months ago

Hey @acao, thank you for looking into this and providing a working codesandbox! I quickly ramped up a repo to show you exactly how our system works and what tech we are using. You can find it here: https://github.com/henrikhertler/codemirror-repro

I marked the line you need to comment in for seeing an issue: https://github.com/henrikhertler/codemirror-repro/blob/b0f742046b60ef5bc238a51111bc623bb12b33e6/pages/index.tsx#L61C36-L61C36

It is not exactly the error @honigbienlein provided, but they are probably related. If we could make the minimal repro work, I guess we can make the system work or at least improve your extension to work with @uiw/react-codemirror.

If you need anything more, please let us know!

acao commented 8 months ago

thank you @henrikhertler! is this the error that you're referring to?

image

it's the one that appeared in your repro when I uncommented the extension. The one @honigbienlein shared would imply our library has an issue, however this error implies the dependency modules are an issue, so they may be related but the implications of each bug are different. noting that next standalone mode is being used is a big factor! @honigbienlein are you using standalone mode with next as well?

I can reach out to the maintainers of this library if so, or perhaps they fixed this in the latest 8.x version of their library which I'm releasing support for tonight!

The other alternative would be to bundle the dependencies of the library for npm which is common and popular in js/ts community, but I try to avoid because it creates security vulnerabilities

henrikhertler commented 8 months ago

Thank you for providing a version 0.5.0!

@acao yes, that is the error. I just added your package without looking at the version – I already use 0.5.0 in my repro. If you downgrade to 0.4.0 you will get exactly the error @honigbienlein provided earlier. :) I created a branch you can checkout: https://github.com/henrikhertler/codemirror-repro/tree/cjs-0-4-0 So I'm pretty sure she will get the same error if she upgrades, since the setups a identical.

About the standalone – I'm not really familiar with it. Feel free to remove it during your testing from the repro. It might not be needed for the project.

acao commented 8 months ago

@henrikhertler thank you for this! interesting and perhaps promising to see that change led to different errors. I hope to get to it by tomorrow.

I just have an instinct about standalone mode so I would suggest seeing what happens if you disable it and remove .next just to be certain.

Recently on a project this summer there had been a tooling setup that required using a docker image for next 13 w/ app dir, and in that case standalone mode makes sense.

However, it caused many issues, so many that we switched to running in locally without docker so we could disable mode: standalone. It does some heavy lifting with node_modules which is why it raised my eyebrows for this issue

https://nextjs.org/docs/pages/api-reference/next-config-js/output

I will let you know what I learn by tomorrow! But let me know if just removing output: standalone thus opting for the default works for you both locally and in your deployed stage/preview/etc environment.

henrikhertler commented 8 months ago

@acao, thanks for sharing your experiences

So I removed output: 'standalone' and deleted .next, yarn.lock and node_modules (just to be sure).

On version 0.4.0: I received the same error as before. Server Error Error: Cannot find module '/Users/henrikhertler/work/bauder/codemirror-repro/node_modules/codemirror-json-schema/dist/json-completion' imported from /Users/henrikhertler/work/bauder/codemirror-repro/node_modules/codemirror-json-schema/dist/index.js

On version 0.5.0: Also the same error. Server Error SyntaxError: Named export 'getTypeOf' not found. The requested module 'json-schema-library' is a CommonJS module, which may not support all module.exports as named exports. CommonJS modules can always be imported via the default export, for example using: import pkg from 'json-schema-library'; const { getTypeOf, isJsonError, reduceSchema, } = pkg;

I did run it locally only for now. I'm looking forward to your findings tomorrow!

honigbienlein commented 8 months ago

@acao Yes, I am using standalone mode with next as well. So I removed output: 'standalone' and deleted .next, and still get the same error..

I'm curious about what you found out tomorrow :)

acao commented 8 months ago

I had a bit of time to look into it some more, and i suspect it's perhaps related to the way we currently import directly from internal third party /dist/ paths in utils/schema-lib/ and usage of type module.

I published a canary of a new WIP branch that will drop this forked internal library for the new version of the third party library which fixes this issue. there is one regression with displaying description on hover, but if either of you want to give this a try that would be helpful!

codemirror-json-schema@0.5.1-canary.0

henrikhertler commented 8 months ago

I had a bit of time to look into it some more, and i suspect it's perhaps related to the way we currently import directly from internal third party /dist/ paths in utils/schema-lib/ and usage of type module.

I published a canary of a new WIP branch that will drop this forked internal library for the new version of the third party library which fixes this issue. there is one regression with displaying description on hover, but if either of you want to give this a try that would be helpful!

codemirror-json-schema@0.5.1-canary.0

I tried it and got the following error now: Screenshot 2023-11-09 at 15 58 04

acao commented 8 months ago

Ok! Thanks - this now clarifies that it is indeed an issue with our third party module, but they are great to work with and I will be happy to open a PR to add esm support to their library

henrikhertler commented 8 months ago

Hey @acao, did you find time to open a PR in the mentioned third-party module? If yes, could you be so nice to share the link so that we could follow the current status? Please feel free to contact us if we can help you with anything.

acao commented 8 months ago

I have not had a chance unfortunately, quite busy with a job search, I will ask about it this week. If you want to, you could open a ticket for us, just asking for full esm support and thus asking if they can enable type: module. Opening a PR for them would help as well, and would be a way to locally test if this fixes the issue

henrikhertler commented 8 months ago

I have not had a chance unfortunately, quite busy with a job search, I will ask about it this week. If you want to, you could open a ticket for us, just asking for full esm support and thus asking if they can enable type: module. Opening a PR for them would help as well, and would be a way to locally test if this fixes the issue

No worries and good luck with your job search! I opened an issue on the repo: https://github.com/sagold/json-schema-library/issues/51

honigbienlein commented 7 months ago

@acao Do you have any updates for us? Kindly inform us if this requires more time. Then we need to find another library or create a solution on our own. We are a bit stuck with our project, but I don't want to worry you.

acao commented 7 months ago

Hi there - yes I had some time to experiment and it seems that json-schema-library needs to ship with esm support to solve this issue

(that is, unless the latest release of our library solves the issue for you)

acao commented 7 months ago

there might also be a workaround with next.js and/or webpack itself - try this with json-schema-library:

https://nextjs.org/blog/next-13-1#built-in-module-transpilation-stable

luisgustavosc commented 5 months ago

I'm using nextjs v13.4.7 and the module transpilation worked for me but with codemirror-json-schema

honigbienlein commented 5 months ago

@acao Hope you're doing fine. I can see a PR for supporting esm: https://github.com/sagold/json-schema-library/pull/30 Were you able to review and check?

acao commented 5 months ago

@honigbienlein hi thanks, yes I saw that PR last year! as the maintainer mentioned, its quite outdated, but someone could start a new PR and re-create their config changes. maybe i will get to it later today or this week. I'm very busy currently as I'm working on contract on another open source project in addition to my day job, but I don't want this project to suffer because of that!

update: I think I've found how to fix your example from before where I could reproduce the error, using transpileModules!

acao commented 5 months ago

here is your fix PR @henrikhertler and @honigbienlein !! https://github.com/henrikhertler/codemirror-repro/pull/1 it seems transpilePackages works fine here. for whatever reason I needed to include both to get it to work. This is not ideal, and ultimately the esm PR on json-schema-library's end will resolve this issue for us

henrikhertler commented 5 months ago

Awesome @acao! I tested it and it works for my use case. Thank you a lot for your help! All I can say: dj khaled

acao commented 5 months ago

music to my ears! glad to help, and thank you for helping make our library better! I will include something in the readme about this

honigbienlein commented 3 months ago

@acao I apologize for the delay in my reply. I tested it as well, and it now works I really appreciate your help! :)

andreyqin commented 1 month ago

@acao Hi! It seems like I have also run into this issue in my project. I'm also using @uiw/react-codemirror but instead of Next.js I have a Webpack setup. Do you know of any other alternatives of transpilePackages for projects build with Webpack only? Or how can I even work around the issue? Would really appreciate of any help on this!

andreyqin commented 1 month ago

Well, after having some deep dive session it looks like I've found a workaround that seems be work for me. What I did is just added a module resolver for codemirror-json-schema and set the fullySpecified webpack's setting to false. After that my project got compiled without the error:

module.exports = {
  // ...
  module: {
    rules: [
      {
        test: path.resolve(__dirname, 'node_modules/codemirror-json-schema'),
        resolve: {
          fullySpecified: false,
        },
      },
    ],
  },
};