-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
Move types into namespace so that they can be imported #258
base: master
Are you sure you want to change the base?
Conversation
@@ -1,6 +1,52 @@ | |||
import { EventEmitter } from "events"; | |||
|
|||
interface BasePromptOptions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This diff looks a bit odd b/c for git the smallest way to express the change is to actually move the start of the namespace and the classes above it to the top of the file, rather than moving the interfaces into the namespace.
So the visualisation of this diff is inverse to it's result :)
@doowb could I get a review on this? (I can't request reviews anymore due to the age 😬) |
@G-Rath thanks for the PR. Sorry for not responding sooner. There are a lot of TypeScript related PRs and since I'm not as familiar with TypeScript, I've been trying to make sure I understand what type of affects the changes might have. From your initial description, this one looks like it will just "add" functionality by exposing more of the Types and Interfaces through the I just merged in #266, which updated Thank you. |
There's an existing PR: #82. However, it's still not merged. |
Exactly. This isn't a breaking change :) |
@doowb I've rebased this PR - let me know if you'd like any other changes :) |
@jonschlinkert @doowb any chance of getting this reviewed & merged? While it sounds like #307 is going to greatly improve on things, it's a big PR that could require work for apps to update to when it is finally released. Meanwhile this change is very simple, not breaking, and so should provide a good middle ground to improve TS support while waiting for #307 to land :) |
We’ll review asap
…Sent from my iPhone
On Oct 18, 2020, at 7:39 PM, Gareth Jones ***@***.***> wrote:
@jonschlinkert @doowb any chance of getting this reviewed & merged? While it sounds like #307 is going to greatly improve on things, it's a big PR that could require work for apps to update to when it is finally released.
Meanwhile this change is very simple, not breaking, and so should provide a good middle ground to improve TS support while waiting for #307 to land :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@jonschlinkert any update? |
Sorry for the delay, I'l merge and publish asap. |
@jonschlinkert I hate to be a pain, but any chance of getting this merged & released? I'm kicking off a new TypeScript project that I'd like to use enquirer for, but without this change it's making it pretty hard |
It's been over a year :( |
This allows access to the types without being a breaking change.