Add type definitions via index.d.ts

[steveszc]

This PR adds a comprehensive index.d.ts that adds type definitions for all public classes in ember-simple-auth. This types are currently in-use internally at CrowdStrike.

While this approach has the benefit of finally landing "official" types, these types must be hand maintained as changes are made to the implementation itself. A better approach might be native TS implementation or JSDoc types, but this is better than nothing and will give ember-simple-auth a base of types to build upon and improve.

fixes #2064

[RobbieTheWagner]

This is a huge step in the right direction, thank you @steveszc!

[BobrImperator]

Sorry to bring it up only now @steveszc 🙏

I myself am not well informed about consuming Typescript addons in ember projects. So I went to setup a project to test this out and I had to specifically import the types in my types/global.d.ts file.

Could you please document the steps to include ember-simple-auth types in user projects?

Another question would be - would it make sense in your opinion, to make SessionService<T> argument optional? At the moment users will be forced to provide the type and carry it around e.g.

import SessionService from 'ember-simple-auth/services/session'; 

interface SessionData {
  authenticated: Record<string, unknown>
}

export default class Application extends Controller {
  @service declare session: SessionService<SessionData>;  
}

This particular problem potentially isn't huge because the session service usually is abstracted or extended, however that'd also need to be documented.

Would you consider it the expect API?

Or maybe the recommendation should be to extend the ESA service instead?

// user land
// app/services/session.ts
import EsaSessionService from 'ember-simple-auth/services/session';

type Data = Record<string, unknown>;

export interface SessionData {
  authenticated: Data;
}

export default class UserLandSessionService from EsaSessionService<SessionData> {}

// DO NOT DELETE: this is how TypeScript knows how to look up your services.
declare module '@ember/service' {
  interface Registry {
    'session': UserLandSessionService;
  }
}

import Component from '@glimmer/component';
import SessionService from 'user-land/services/session';
import { service } from '@ember/service';

export class MyComponent extends Component {
  @service declare session: SessionService;
}

[RobbieTheWagner]

In general, types should be imported where they need to be consumed. I think we would typically import them in specific files, where needed, rather than globally.

[colenso]

In general, types should be imported where they need to be consumed. I think we would typically import them in specific files, where needed, rather than globally.

I second this ☝️ As for:

Could you please document the steps to include ember-simple-auth types in user projects?

The steps I followed are:

1. Include it in the tsconfig.json under paths like so:

"paths: {
    "ember-simple-auth/*": [
        "node_modules/ember-simple-auth"
      ],
}

2. Import the definition for the type that you need where you need it. For eg: If I have a session-auth.js service that uses the session service offered by ember-simple-auth, I'd import it and use it something like this:

session-auth.js
import { inject as service } from '@ember/service';
import Session from 'ember-simple-auth/services/session';

export default class SessionAuthService extends Service {

    @service declare session: Session<{}>

}

@BobrImperator would be nice if you could give this another look. I'm working on migrating our app to typescript and this PR would be a real help.

[BobrImperator]

Thanks for participating in the discussion 💯 This makes sense to me 👍

I'd like ESA to include the recommended setup steps in the readme and we should be ready to go.

[steveszc]

I'm happy to update the readme with some instructions in this PR.

[colenso]

@steveszc Would be nice to get this one in. I added some docs in https://github.com/steveszc/ember-simple-auth/pull/1. Please 🙏 review and merge so @BobrImperator can add these type defs in.

[steveszc]

@colenso I provided some feedback here. A couple changes are needed to get it merged.

[colenso]

@colenso I provided some feedback here. A couple changes are needed to get it merged.

I made the requested changes @steveszc Please merge if it's OK and then @BobrImperator can merge this PR.

[RobbieTheWagner]

Any chance this could be merged soon?

[GerritSommer]

+1

[BryanCrotaz]

Any news?

[NullVoxPopuli]

anything holding this up?

[RobbieTheWagner]

@BobrImperator @marcoow any chance we could merge this? It would be super helpful!

[BobrImperator]

From my perspective this seems fine, the provided types look good as well.

This needs to be rebased and then adapted for the V2 addon structure. Where as I understand the index.d.ts will need to be copied to dist/ during build.

Would be nice to have a guide directly in the readme on how to enable the types in user projects. But that can be added separately.

[NullVoxPopuli]

Where as I understand the index.d.ts will need to be copied to dist/ during build.

This isn't needed. To have the d.ts picked up, it only needs:

• to be present in the npm package (covered by package.json#files or the inverse of .npmignore)
• and have types/exports.*.types point at the d.ts file.
  • or be named index.d.ts in the root of the package (index files are automatically identified)