Aliasing an exported type as the default export hides needed types

[shellscape]

Referencing the way that this project exports the public members here:

samlify/index.ts

Line 2 in 728fde8

export { default as IdentityProvider } from './src/entity-idp';

By using IdentityProvider as the default export alias, the actual IdentityProvider class (https://github.com/tngan/samlify/blob/master/src/entity-idp.ts#L30) is being hidden from resolution by consuming code.

If we're trying to build say, a cache that contains one or more IdentityProvider class instances, and type that cache properly, there's no way to reference that type.

We'd be forced to use a module file path:

import { IdentityProvider } from 'samlify/types/src/entity-idp';
import { ServiceProvider } from 'samlify/types/src/entity-sp';

and that's really really bad since that approach is well known to be prone to breaking between patch versions.

A better solution here would be to export such like:

export * from './src/entity-idp';

and export a named function in entity-idp.ts:

const createIdentityProvider = (props: IdentityProviderSettings) => {
  return new IdentityProvider(props);
}

export default createIdentityProvider;

Or, at the very least, export the types for IdentityProvider and ServiceProvider in a minor version without breaking changes.

[tngan]

@shellscape We better get rid of introducing breaking change. I already have a plan to release v3 by the end of this year, by simplifying the logic, and have better API design.

What we can do for now is to export something like

// index.ts
import IdentityProvider, { IdentityProvider as IdentityProviderInstance } from './src/entity-idp';
import ServiceProvider, { ServiceProvider as ServiceProviderInstance } from './src/entity-sp';
// ...
export {
  IdentityProvider,
  IdentityProviderInstance,
  ServiceProvider,
  ServiceProviderInstance
}
// samlify.IdentityProvider won't break with this approach, it's still calling the default function

For the cache, it can be something like as follow.

type Cache = {
  [issuer: string]: IdentityProviderInstance | ServiceProviderInstance
};
let cache: Cache;

Any better idea?

[FreifeldRoyi]

Any update on this issue? This is actually preventing me from migrating to Samlify from another lib

[tngan]

@shellscape @FreifeldRoyi How's the above mentioned proposal? This wouldn't break the current typing definition.

[FreifeldRoyi]

Sounds good

[natevecc]

@FreifeldRoyi as a work around you can use ReturnType

import { IdentityProvider, ServiceProvider } from 'samlify';

let idp: ReturnType<typeof IdentityProvider>;
let sp: ReturnType<typeof ServiceProvider>;

[tngan]

@FreifeldRoyi @shellscape Feel free to check the release of 2.7.4.