rhinobase / hono-openapi

A plugin for Hono to generate OpenAPI Swagger documentation
MIT License
247 stars 5 forks source link

The export types are still wrong #13

Open marcomuser opened 2 days ago

marcomuser commented 2 days ago

Unfortunately, the recent release has not fixed the issues affecting Node.js projects using ESM with tsconfig "module": "node16" or "module": "nodenext". Note that these are the officially recommended settings for all Node.js projects. Please check the awesome website from Andrew Branch of the TS-team which highlights the current issue: https://arethetypeswrong.github.io/?p=hono-openapi%400.2.0.

From my understanding we need to make sure that the output files for ESM have a .mjs file extension (and its type files the matching .mts file extension). Currently, this package declares itself as a CommonJS library in it's package.json so that all the exported files with .js extensions will always be interpreted as CJS modules by Node.js.

I tried to fix the output settings in the rollup config but nx is magically trying to take over with it's withNx function and I was not able to change the settings to something like this:

output: [
    {
      dir: './dist',
      format: 'esm',
      entryFileNames: '[name].mjs',
    },
    {
      dir: './dist',
      format: 'cjs',
      entryFileNames: '[name].js',
    }
  ],

As a side note: I'm not sure why anyone would still build new projects in 2024 with CJS. That ship has long sailed. The amount of complexity one has to handle to make a dual-emit properly work is unfortunately considerable. I would simply consider whether it is not time to jump on the standard bandwagon and embrace the ESM happy path. :)

riderx commented 1 day ago

Same issue i cannot use it in deno because if that the typing is broken.

MathurAditya724 commented 16 hours ago

Thanks for the detailed feedback! I do plan to drop CJS support eventually, but since hono still supports it, we need to align for now. I'll revert to match hono's structure for the time being, which should resolve these issues.

marcomuser commented 14 hours ago

Hono also has problems with its dual-emit. Although one difference is that hono has type: "module" in its package.json which makes it a esm package. Since most people use esm I suspect it gets way less complaints about this specific issue even if it doesn't work properly in all cases either.

I would recommend looking at the tanstack nx monorepo for a functioning dual-emit. They use tsup instead of rollup which makes it easier. Check it out:

MathurAditya724 commented 14 hours ago

What if we change the package type to module? Will that solve the issue? Also, should we revert the exports to hono, or will the current ones work?

I also face this issue in the https://github.com/rhinobase/hono-rate-limiter package so that we can implement the same solution there too

marcomuser commented 12 hours ago

Setting the type to module is definitely the way to go. If you then want to support exactly what Hono supports you can model its exports for sure (as long as that matches your actual dist output). That would make it work for most but not all users. Afaik it would work for esm and bundler projects but not for classic cjs projects.

That's not what I would do but happy with whatever works for you! I would set the type to module, release a esm build, remove the cjs exports and communicate clearly that this is a esm only library: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c