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

Add type definitions for bottleneck/light #160

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

copperwall
Copy link
Contributor

Hey there, octokit/plugin-retry.js uses bottleneck/light for ratelimiting, but there's a little bit of trouble importing types when using bottleneck/light over the main bottleneck import.

I did some testing and adding a light.d.ts definition file with existing namespace and class types but under a bottleneck/light module lets typescript find the types for the light import.

I noticed that the existing type definitions were using EJS templates, so I move the namespace and class type definitions to a shared EJS template and included that in both the bottleneck.d.ts.ejs and light.d.ts.ejs template files.

This fixes the issue in octokit/plugin-retry.js#110, but I'm open to suggestions if there's a better way to do this!

Copy link

@gr2m gr2m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me 👍🏼

@SGrondin SGrondin merged commit b835283 into SGrondin:master Jul 21, 2020
@SGrondin
Copy link
Owner

I'll make a new release tonight

@gr2m
Copy link

gr2m commented Jul 28, 2020

Hey Simon, could you do the release please?

@oscard0m
Copy link

Hi @SGrondin, I was walking through existing open issues in Octokit and I arrived to an issue which depends on these changes to be released, what's the status of bottleneck project? Can we help you with something?

Thanks for your time and please let us know if you need help.

@gr2m
Copy link

gr2m commented Jun 2, 2023

@SGrondin it looks like there is still no new release. Could you please do a new release that includes this fix?

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

4 participants