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

Inquirer 10/9 compatibility problems. #1527

Closed
5 tasks done
mshima opened this issue Aug 30, 2024 · 14 comments
Closed
5 tasks done

Inquirer 10/9 compatibility problems. #1527

mshima opened this issue Aug 30, 2024 · 14 comments

Comments

@mshima
Copy link
Contributor

mshima commented Aug 30, 2024

Inquirer 10 is designed to be backward compatible with Inquirer 9.
That's great and allows yeoman-generator/environment stack to adopt Inquirer 10.

Problems:

Those issues needs to be addressed for yeoman-environment to officially adopt it.

@SBoudrias
Copy link
Owner

Do you have a full-repro for the message type issue? I don't think it always fail, so I wonder if it's contextual to how the message is passed down somehow...

SBoudrias added a commit that referenced this issue Sep 2, 2024
Ref #1527

------------

Co-authored-by: Simon Boudrias <admin@simonboudrias.com>
@mshima
Copy link
Contributor Author

mshima commented Sep 2, 2024

Anoter issue: ui.close() does not work with modern prompts and there is no alternative.

@SBoudrias
Copy link
Owner

I didn't look too much into it yet, but I think filter might not work at all anymore - not sure when it went (maybe it was initially implemented in prompt classes 😮‍💨). But that'll be a major backward compat issue to fix.

Ref #1540

@SBoudrias
Copy link
Owner

Hey Marcelo, anything that's still a blocker? (I'm not sure how accurate the list in the ticket is after the work we put in this 😄)

@DelliriumX
Copy link

I will ride in on this and add, since TS was not supported in v9, we used @types/inquirer. The interfaces that package exported compared to the now TS ready version is very different. Now, i dont mind changing a few things, afterall, it was a major up, but you no longer export types, other than QuestionMap no type is exported from inquirer. How are we suppose to type stuff?

I want to make an array of questions which I will later put into an inquirer.prompt(...my array) what do I do? How do I type my array when the "Question" type is not exported by the package? We used to have DistinctQuestion in @types/, i now find something similar with AnyQuestion but it is not exported.

@mshima
Copy link
Contributor Author

mshima commented Sep 17, 2024

@DelliriumX if you want to add your question to types, you can implement like this:

declare module './src/index.mjs' {
interface QuestionMap {
stub: { answer?: string | boolean; message: string; default?: string };
stub2: { answer?: string | boolean; message: string; default: string };
stubSelect: { choices: string[] };
failing: { message: string };
}
}

If you want to wrap inquirer api and expose a customized api like myself, then you need to extract the desired api which will make you rely on unstable interface.

@mshima
Copy link
Contributor Author

mshima commented Sep 17, 2024

Hey Marcelo, anything that's still a blocker? (I'm not sure how accurate the list in the ticket is after the work we put in this 😄)

Not exposing question types is still a problem to me.
I cannot easily support rxjs questions alternative, we currently only expose array or single question so we can inject data in questions.
I am looking to alternative way to implement.

@SBoudrias
Copy link
Owner

I'm open to exporting the question type; but note that it prevents inferring the return type. So I'm just unclear in which case that is positive.

Does it not work with const questions = [] as const; or const questions = [{} as const, {} as const, ...]?

@DelliriumX
Copy link

@DelliriumX if you want to add your question to types, you can implement like this:

If you want to wrap inquirer api and expose a customized api like myself, then you need to extract the desired api which will make you rely on unstable interface.

Thing is, i do not, I want to be able to "type" my own object structure that fits a "question" with a type like we used to be able to do prior (v9 it was using the DistinctQuestion type from @types/inquirere). Essentially:

const obj1 = { /* ... */}       // this is untyped and has no intelisense, shape or anything else

inquirer.prompt([
   {
     /* this object literal is recognized as a question and it has all the benefits*/
     /* I want to apply this internal type to my obj1 object*/
    }
])

This is used to be able to construct question prompts dynamically by modifying what is the contents of the array

@DelliriumX
Copy link

DelliriumX commented Sep 18, 2024

Does it not work with const questions = [] as const; or const questions = [{} as const, {} as const, ...]?

These are not questions, these are plain objects, there is literally 0 type-safety or intelliSense support here. They will give a proper return type when used in context of a prompt, but that would be the case if they were explicitly typed as well. They however will not be "seen" as questions until they are put inside a prompt. This is essentially what I want, to be able to define a question outside of the context of a prompt.

I'm open to exporting the question type; but note that it prevents inferring the return type.

I wont claim to understand the exact types you are using and why this would be the case, but I do not think this is correct, for two reasons:

  1. exporting a type doesn't prevent it being used the same way it was before, so inference still works for people that do not use it, that should be a given - this makes the statement as such not correct (but that is being nitpicky), the more important reason is:
  2. i do not think that having an question array typed as questions can EVER prevent inference. They are typed as questions even when you do not do it externally, at best you would get a type error if types mismatch. To put it extremely simplified, prompt expects an array of question objects, when you make those objects inline, everything works. It should work even when you make them outside of a function call and pass them to a function.

What i mean by this is:

// this works, for all intents and purposes
const answer = await inquirer.prompt([{ name: 'my question', type: 'input' }])

// this code does the same thing, including the inferrence of return type
const question = { name: 'my question', type: 'input' } as const
const answer = await inquirer.prompt([question])

The big difference between the two is that in first case, you get the full typescript support while "writing" your question, in the second case, you are working with a plain object so anything goes... In an ideal world we would be able to:

// and the QuestionType would give us the necessary TSC support
const question : QuestionType = { name: 'my question', type: 'input' } as const
const answer = await inquirer.prompt([question])

So I'm just unclear in which case that is positive.

When you are creating dynamic questions arrays you cannot have it be done inline inside a prompt. To give an actual example - say you have a QuestionsCollection which is a collection of functions that generate Question objects. You could then "compile" your question prompt by calling these functions inline, to generate a dynamic array of questions. With types still being inferred because from a TSC point of view, these are just objects, like in the examples above. And even if type is not inferred, I'd still prefer to have the option to do so and then provide the generic to the prompt on how I expect the answer to look like - simply because, having the option to dynamically generate a set of questions is that important.

In essence do not force us to do this:

function prompt(...args: Array<{
    title: string,
    type: string | number | boolean
}>): void

type ExtractQuestionType = typeof prompt extends (...args: infer R) => any ? R[0] : never

declare const MyQuestion: ExtractQuestionType // now has TS support

@SBoudrias
Copy link
Owner

@DelliriumX do you want to send a PR + a test (expectTypeOf()...) to make sure we don't break this going forward?

@DelliriumX
Copy link

@DelliriumX do you want to send a PR + a test (expectTypeOf()...) to make sure we don't break this going forward?

I'd be willing to assist you, but I'd rather not take charge on this issue since I am not familiar enough with your entire system to feel confident I grasp it well. Not to mention that I am absolutely horrendous when it comes to writing testing... I've got over 15 years of professional experience in this industry and I've not written more than 10 across my entire career. I'd rather avoid that not to f it up.

@SBoudrias
Copy link
Owner

inquirer@11.1.0 includes the base types.

@mshima
Copy link
Contributor Author

mshima commented Sep 30, 2024

Issues have been resolved.

FYI:
DistinctQuestion is far more restrictive then v9 types.
It requires type checking, this won’t work:

const question = { type: 'input', … };
prompt(question); // question is not valid because type is a generic string

workaround is to inline:

prompt({ type: 'input', … });

or maybe use satisfies;

const question = { type: 'input', … } satisfies DistinctQuestion;
prompt(question);

@mshima mshima closed this as completed Sep 30, 2024
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

No branches or pull requests

3 participants