feathersjs-ecosystem / feathers-swagger

Add documentation to your FeatherJS services and feed them to Swagger UI.
MIT License
226 stars 63 forks source link

Feature: ESM module #253

Open AshotN opened 1 year ago

AshotN commented 1 year ago

Is your feature request related to a problem? Please describe. I am attempting to move my code base off of CJS and onto ESM.

Describe the solution you'd like For feathers-swagger to support both ESM and CJS with conditional exports

Describe alternatives you've considered Default exports are compatible across ESM and CJS, but it is kind of a pain as far as developer experience goes

Additional context I recently updated my package to support both, as an example https://github.com/AshotN/adminjs-feathers/commit/69a536030d85fa05fae83d78fa1e258e94e34466

Mairu commented 1 year ago

feathers-swagger is quite an old module written in plain javascript not using any transpiler.

Can you explain to me the pain of importing cjs modules into esm code?

Mairu commented 1 year ago

@AshotN Can you test the branch https://github.com/feathersjs-ecosystem/feathers-swagger/tree/esm if it does work like that?

AshotN commented 1 year ago

feathers-swagger is quite an old module written in plain javascript not using any transpiler.

Can you explain to me the pain of importing cjs modules into esm code?

Having to import like this

import feathersSwagger from 'feathers-swagger'
const { createSwaggerServiceOptions } = feathersSwagger

@AshotN Can you test the branch https://github.com/feathersjs-ecosystem/feathers-swagger/tree/esm if it does work like that?

Mostly everything worked, besides the functions are not available when default importing.

Have to do this

import swagger, { customMethodsHandler, swaggerUI } from 'feathers-swagger'

because

swagger.swaggerUI is undefined

Mairu commented 1 year ago

Mostly everything worked, besides the functions are not available when default importing.

Have to do this

import swagger, { customMethodsHandler, swaggerUI } from 'feathers-swagger'

because

swagger.swaggerUI is undefined

This was on purpose, I thought this is how it should be to allow "tree shaking" even if it is not a real thing on the backend, but then it is a breaking change, mhh.

Btw in my Typescript projects import { createSwaggerServiceOptions } from 'feathers-swagger'; already worked before, why do you have to split this? -> https://github.com/Mairu/feathersjs-swagger-tests/blob/feathers-v5-koa-typebox/src/services/users/users.ts#L4

AshotN commented 1 year ago

That works because you are not using ESM.

https://github.com/Mairu/feathersjs-swagger-tests/blob/feathers-v5-koa-typebox/tsconfig.json#L7

If swagger.swaggerUI is meant to be undefined. Then the types should be fixed for it

AwesomeBobX64 commented 6 months ago

Chiming in on this old issue. I'm trying to use esbuild for my feathersjs app and feathers-swagger is trying to import @featuresjs/express/rest in a way maybe it was not intended to:


✘ [ERROR] Could not resolve "@feathersjs/express/rest"

    node_modules/feathers-swagger/lib/custom-methods.js:13:46:
      13 │   const { HTTP_METHOD, httpMethod } = require('@feathersjs/express/rest');
         ╵                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~

  You can mark the path "@feathersjs/express/rest" as external to exclude it from the bundle, which
  will remove this error and leave the unresolved path in the bundle. You can also surround this
  "require" call with a try/catch block to handle this failure at run-time instead of bundle-time.

Could this possibly be fixed by the same issue or is it unrelated?

Mairu commented 3 months ago

Hi @AwesomeBobX64, sorry for the late response.

I don't think it is related. This is an export only available in feathers v4 and feathers-swagger supports v4 and v5 and checks on runtime which feathers version is installed by checking the available dependency.

And there is a try/catch checking done before in the if block.

https://github.com/feathersjs-ecosystem/feathers-swagger/blob/657ab29c9f470b0341805b193791adbeeb1693df/lib/custom-methods.js#L11-L13

https://github.com/feathersjs-ecosystem/feathers-swagger/blob/657ab29c9f470b0341805b193791adbeeb1693df/lib/helpers.js#L52-L62

Were you able to mark it as external to resolve your issue?