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.

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

Convert to TypeScript #20

Merged
merged 13 commits into from Jun 10, 2019
Merged

Convert to TypeScript #20

merged 13 commits into from Jun 10, 2019

Conversation

fregante
Copy link
Owner

@fregante fregante commented May 30, 2019

cc @nickytonline if you wanna take a look. The API hasn't changed yet, but we have a chance to fix the type of Options now, as talked about in refined-github/refined-github#1780 (comment)

Most I'd like to know if there's a way to pass typeof defaults through to the defaults — this may be easier once the constructor accepts {defaults}

Problem to keep in mind: microsoft/TypeScript#5341

@nickytonline
Copy link
Contributor

I'll take a look at this this weekend.

@nickytonline
Copy link
Contributor

Looking at it, I think everywhere you have OptionsSync.Options, can be replaced by T = OptionsSync.Options and use T wherever OptionsSync.Options is used for types. I think this makes sense unless I'm missing something since anyone that consumes this package will have their own set of options. Happy to chat more about this whenever you're free.

e.g.

getAll<T = OptionsSync.Options>(): Promise<T> {
   ...
}

@fregante
Copy link
Owner Author

fregante commented Jun 3, 2019

Is there a way to have the types associated with the instance instead of having each method generic?

Also because T should always match typeof Definitions.defaults (which will be provided in the constructor after 28bc67f, separate branch)

@nickytonline
Copy link
Contributor

I don't think you can get away without generics. I'd have to mess around with this a bit more once you're other branch is merged or up for review. Let me know when that is up or merged and I'll find some time to tinker.

@fregante
Copy link
Owner Author

fregante commented Jun 10, 2019

It looks like I can have a “generic class”, so the instance will return T = defaults, instead of specifying the type in the getter setter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants