Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TypeScript definitions for injected session service #2064

Open
allthesignals opened this issue Dec 16, 2019 · 11 comments · May be fixed by #2514
Open

TypeScript definitions for injected session service #2064

allthesignals opened this issue Dec 16, 2019 · 11 comments · May be fixed by #2514

Comments

@allthesignals
Copy link

I'm trying to import this service:

import EmberSimpleAuthSession from 'ember-simple-auth/addon/services/session';

So that I can use it as a type:

export default class ShowProjectController extends Controller {
  @service
  session!: EmberSimpleAuthSession

...according to the e-typescript documentation.

But TypeScript can't find the definitions. So, I was able to pull this in and have better luck:
https://github.com/Gavant/octane-bootcamp/blob/master/types/ember-simple-auth/services/session.d.ts

But, I'd hate for this file to fall out of sync to any changes across versions.

Separately, I know there's some churn on how to deal with Mixins, but we can at least type parts of the addon. Is there something I'm missing?
@RobbieTheWagner
Copy link
Contributor

It's been a year on this issue. Any updates here?

@yavuzitconsulting
Copy link

just so you guys now,
this is still an up-to-date issue in 2021 :)

steveszc added a commit to steveszc/ember-simple-auth that referenced this issue Sep 24, 2021
@steveszc
Add type definitions
3ef4e0c 
fixes mainmatter#2064
steveszc added a commit to steveszc/ember-simple-auth that referenced this issue Sep 24, 2021
@steveszc
Add type definitions
f1972a5 
fixes mainmatter#2064
@steveszc
Copy link

steveszc commented Sep 27, 2021 •
edited

I'd be interesting in contributing type definitions for ESA if there is interest from the maintainers.

@marcoow @BobrImperator Any opinions on how best to add type definitions?

An index.d.ts file would be the most straightforward to add but might end up being difficult to maintain.
Since there is already extensive JSDoc we could look at generating type definitions from the JSDoc comments, but this will require some new CI tooling to make sure the JSDoc types are correct. I also have concerns that the base class implementations often don't match the JSDoc comments (since the JSDoc represents how subclasses should implemented, rather than representing the implementations of the base classes). Fixing that particular issue may require a decent amount of code changes to the base class implementations.

@steveszc
Copy link

There is also the possibility of full typescript conversion of the addon, although I'm not sure if there would be interest in that from the maintainers.

@yavuzitconsulting
Copy link

2023, still an open issue, anyone have the solution yet?

@RobbieTheWagner
Copy link
Contributor

@marcoow any plans to add types?

@marcoow
Copy link
Member

marcoow commented Feb 8, 2023

I don't think anyone on our side is currently planning to do this tbh but we'd be happy to merge a PR!

@RobbieTheWagner
Copy link
Contributor

Perhaps someone could PR in the Gavant types linked above?

@steveszc
Copy link

steveszc commented Feb 8, 2023

I have a much more robust set of types we've been using internally at CrowdStrike.
I'll see if I can get those PR'd as an index.d.ts

steveszc added a commit to steveszc/ember-simple-auth that referenced this issue Feb 8, 2023
@steveszc
Add type definitions
0e8627e 
fixes mainmatter#2064
steveszc added a commit to steveszc/ember-simple-auth that referenced this issue Feb 8, 2023
@steveszc
Add type definitions
e2bc049 
fixes mainmatter#2064
steveszc added a commit to steveszc/ember-simple-auth that referenced this issue Feb 8, 2023
@steveszc
Add type definitions
06e27bc 
fixes mainmatter#2064
@steveszc steveszc linked a pull request Feb 8, 2023 that will close this issue
Open
@steveszc
Copy link

steveszc commented Feb 8, 2023

PR open, feedback welcome 😃
#2514

@BobrImperator
Copy link
Collaborator

Thanks for pinging the issue 👍
I've been looking into refactoring the codebase to use tracked and native classes.
I think it could be possible to use typescript at that point or at least do some groundwork into making it easier to type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add type definitions via index.d.ts steveszc/ember-simple-auth
6 participants
@marcoow @RobbieTheWagner @allthesignals @steveszc @BobrImperator @yavuzitconsulting