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

Reserve promise rejections (i.e. async exceptions) for exceptional circumstances #485

Closed
mattbrunetti opened this issue Apr 2, 2017 · 11 comments
Assignees

Comments

@mattbrunetti
Copy link

mattbrunetti commented Apr 2, 2017

With the browser's native prompt function, pressing cancel doesn't result in an error, but instead a null return value. (Similarly, with the native alert function, hitting escape doesn't result in an error, and with the native confirm function, clicking no doesn't result in an error but instead a false return value.)

I believe this to be the desired behavior. Especially when using the up-coming async/await feature of ES (being used already via Babel), the current swal2 behavior leads to some awkward control-flow...

let result
let error
try {
  result = await swal({
    text: 'What is your favorite color?'
    input: text,
    showCancelButton: true
  })
} catch (_error) {
  error = _error
}
if (error) {
  swal('I did not get your favorite color because "${error}".')
} else {
  swal(`Your favorite color is ${result}.`)
}

The following would be a bit nicer.

const result = await swal({
  text: 'What is your favorite color?'
  input: text,
  showCancelButton: true
})
if (result.dismiss) {
  swal('I did not get your favorite color because "${result.dismiss}".')
} else {
  swal(`Your favorite color is ${result.value}.`)
}

@limonte Would you review a PR adding an option to enable this behavior? Let's call the option rejections and have it default to true?

@limonte
Copy link
Member

limonte commented Apr 3, 2017

Please go ahead with a PR 🚀

@toverux
Copy link
Member

toverux commented Apr 3, 2017

Excellent! Using TypeScript, I've already had this problem.

We should also keep that issue somewhere in our mind for the next major, to enable the new behaviour by default (and maybre remove the option and related code).

@mattbrunetti
Copy link
Author

@limonte @toverux Does the same go with the API for the inputValidator method? It should signal a validation error (which is a user error, not a technical/system error) by resolving with the validation message, rather than rejecting with the validation message, right? A falsy resolution value would signal that validation succeeded.

The control-flow is not as bad here, but I think in principal we should not mix user errors with actual technical/system errors.

@mattbrunetti
Copy link
Author

It makes it difficult to debug actual errors thrown inside the function. Also, if there are any errors in production, I want them bubbled up to Rollbar error reporting.

Same goes for rejections in preConfirm; I think they should be treated as real errors instead of a user validation message.

That said, to support validations for the "multiple-input" scenario, could we always call the function, even if input option was not provided?

@limonte
Copy link
Member

limonte commented Apr 7, 2017

Both of your comments make sense. But for beginning please implement the scope of the initial comment only. After reviewing and testing I will be able to decide what to do next. Thanks @mattbrunetti !

@zenflow
Copy link
Member

zenflow commented Oct 20, 2017

const {dismiss, value: favoriteColor} = await swal({text: 'What is your favorite color?', input: 'text'})
if (dismiss) {
  swal(`I did not get your favorite color because "${dismiss}".`)
} else {
  swal(`Your favorite color is "${favoriteColor}".`)
}
const {value: name} = await swal({text: 'Full name?', input: 'text'})
const {value: location} = await swal({text: 'Location?', input 'text'})
await swal(`Hi ${name}, from ${location}!`)

@limonte
Copy link
Member

limonte commented Oct 20, 2017

@zenflow Beautiful, readable and maintainable code! Love it!

@limonte
Copy link
Member

limonte commented Nov 16, 2017

🎉 🎉 🎉 Done in https://github.com/limonte/sweetalert2/releases/tag/v7.0.0

@clopezpro
Copy link

Uncaught SyntaxError: await is only valid in async function

@toverux
Copy link
Member

toverux commented Nov 28, 2017

rude answer to rude message: https://gist.github.com/Rich-Harris/0b6f317657f5167663b493c722647221

@zenflow
Copy link
Member

zenflow commented Nov 28, 2017

Fun fact: With Chrome 62 you can execute the above example code in your console.

image

But otherwise, top-level await is not allowed.

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

5 participants