Feature: ESM module

[AshotN]

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 AshotN/adminjs-feathers@69a5360

[Mairu]

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]

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

[AshotN]

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]

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]

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]

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]

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.

feathers-swagger/lib/custom-methods.js

Lines 11 to 13 in 657ab29

	if (checkImport('@feathersjs/express/rest', 'HTTP_METHOD')) {
	const { activateHooks } = require('@feathersjs/feathers');
	const { HTTP_METHOD, httpMethod } = require('@feathersjs/express/rest');

feathers-swagger/lib/helpers.js

Lines 52 to 62 in 657ab29

	exports.checkImport = function checkImport (packageName, child) {
	try {
	const rest = require(packageName);
	/* __istanbul ignore else: for @feathers/express versions < 4 */
	if (rest[child]) {
	return true;
	}
	} catch (e) {}
	/* __istanbul ignore next: for @feathers/express versions < 4 */
	return false;
	};

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