sagold / json-schema-library

Customizable and hackable json-validator and json-schema utilities for traversal, data generation and validation
MIT License
175 stars 18 forks source link

Request for Full ESM Support and Enabling "type: module" #51

Closed henrikhertler closed 7 months ago

henrikhertler commented 10 months ago

I kindly request the following enhancements in json-schema-library:

  1. Full ESM Support: Enable comprehensive ESM support to ensure seamless integration into projects using ECMAScript modules.
  2. Enable "type: module": We propose enabling the "type: module" field in the package.json to facilitate the use of ECMAScript modules.

For more context, I am coming from this issue: codemirror-json-schema issue 66

tl;dr: We want to integrate @uiw/react-codemirror into our nextjs app and add the extension codemirror-json-schema for linting and code completion. After a short discussion it seems to be possible if your lib supports ESM and type: module.

If feasible, I am open to contributing to this enhancement by opening a PR. However, before doing so, I would appreciate guidance and validation from you to ensure that the proposed changes align with the project's goals and conventions.

sagold commented 10 months ago

We already have an open PR and issue open issue for this request. But I am still uncertain why this is required.

In the dist folder you can see json-schema-library is exported

with this I am able to include it in webpack builds, directly in the browser, use require in node and dynamic import in e.g. node 18: const jlib = await import('json-schema-library')

Can you be more specific on why this is required and offer an example for me to reproduce? In general I am happy to add missing export types, but this should come in addition to the above use cases.

Happy to hear from you.

sagold commented 10 months ago

quickly skimmed the issue you referenced. Let us find a solution to this!

acao commented 10 months ago

FYI: I hope to take a closer look at this during the upcoming week and to determine whether we can do something about it on our side! I think there is one thing I might have overlooked

honigbienlein commented 9 months ago

@acao Do you have any updates?

acao commented 9 months ago

yes sorry for the delay! the latest version of our library is now using the latest version of this library, without any forked elements. I thought the direct imports of the dist directories of this module were the problem but they are not. It seems their bug requires ESM support of this library to be resolved

acao commented 9 months ago

@sagold do you have any updates on adding esm support? CC @honigbienlein

sagold commented 9 months ago

@acao I will add esm support. Possibly within the next two weeks. A following review would be appreciated.

acao commented 9 months ago

@sagold will happily review! I may be able to make a PR this week even to save you the effort, I will leave a comment here when/if I start.

henrikhertler commented 8 months ago

@acao let me know when you have a branch ready and I'll check my use-case if it helps/works. :)

honigbienlein commented 8 months ago

Nice, I can see a PR for supporting esm. Many thanks! @acao Were you able to review and check?

henrikhertler commented 7 months ago

@sagold for my especial use-case it isn't even necessary that your package is ESM ready. Since I use next.js with a version > 13.x @acao found out, that a: transpilePackages: [ 'codemirror-json-schema', 'json-schema-library', ], in the next.config.js is enough.

More about transpilePackages via: https://nextjs.org/docs/app/api-reference/next-config-js/transpilePackages

Feel free to close this issue and thank you for your help!

acao commented 7 months ago

@sagold I would still like to help contribute this eventually, as a way to give back to a library that does so much for us! but again my schedule is wild haha

sagold commented 7 months ago

Thank you all for this discussion. I just tried to find out what actually has to be done/supported and read some articles, e.g. https://blog.isquaredsoftware.com/2023/08/esm-modernization-lessons/ stating the difficulties to get this right at this time. I am pretty sure json-schema-library has less complexity as a react-library as well as far less and less varying consumers. Still, going this direction will very likely bring breaking changes and frustration along the way.

This has to be done at some point, but hopefully we can wait until we have a broader consensus on this topic.

acao commented 6 months ago

we will still be contributing to this effort eventually as it's still causing issues for vite users and other esbuild users who do not have an escape hatch for cjs in node_modules. i know it's very frustrating to deal with this pure esm problem, but I promise I can help you eventually, it will just take time as I am working a full time job whilst also executing a deeply involved OSS contract in another repo. I recently polished up our own esm support and found something super helpful that will save you time.

there are unfortunately multiple paths to full esm support, some involving a lot more work than others depending on the runtime/build context. one of the major factors I've found is moduleResolution, many tutorials use NodeNext or some other option here, but I highly reccomend using Bundler instead. this way, you don't have to rename every import/export to use .js. if you find any tutorials that tell you to rename imports to .js for esm, be wary haha. as i understand, Bundler mode for moduleResolution is also applicable to your situation, of shipping consumable esm libraries to browser or node, and is what we use for shipping esm bundles as well.

this explains it as does the tsconfig docs: https://dev.to/ayc0/typescript-50-new-mode-bundler-esm-1jic

essentially, you can choose to import with extensions, or not provide any extensions at all for imports