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

Implement a queue system or throw an error when prompt is called before previous one has ended. #64

Closed
danielchatfield opened this issue Sep 2, 2013 · 3 comments

Comments

@danielchatfield
Copy link
Collaborator

Migrated from #62 (separate issue that was brought up)

Test case

var inquirer = require('inquirer');

for (var i = 1; i < 5; i += 1) {
  inquirer.prompt([{
    type: 'list',
    name: 'choice',
    message: 'Select:',
    choices: ['one', 'two']
  }]);
}

Currently this just breaks inquirer.

I can't think of any reason why someone would need to call prompt like this as you should just pass multiple questions to prompt and if you need to split it up (you need to calculate some questions based on previous answers and the calculation includes async calls so when will not work) then you would need to do it the proper way (from within the callback).

I propose two options:

Option 1

Throw an error if inquirer.prompt is called before a previous call has finished (removed event listeners and called callback).

Option 2

Implement a queue: https://github.com/pull-requests/Inquirer.js/compare/queue

This isn't currently working properly (some tests hang - not sure why), but you get the general idea.

Option 1 would be dead simple to implement and I feel that option 2 is just helping people who haven't grasped how async works and aren't using inquirer properly anyway. Part of me thinks that it would be better for them if we throw an error as they will then understand that they need to use callbacks and can't just call them serially.

@mpal9000
Copy link

mpal9000 commented Sep 2, 2013

I think the only reason for implementing option 2, is having a synchronous api too. It could be useful when writing shell style scripts (with shelljs for example), in which you target on using sync only functions, for "simplicity".

Personally I like using async style as much as I can. So classifying this as a misuse and reporting it, seems right to me.

@danielchatfield
Copy link
Collaborator Author

@mpal9000 Completely agree

@SBoudrias
Copy link
Owner

I don't believe the flow control belong in Inquirer. This is a totally different concept and responsibility.

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

No branches or pull requests

3 participants