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

[CS2] Async/await documentation; fix for Underscore templating #4351

Merged
merged 8 commits into from
Dec 20, 2016
Merged

[CS2] Async/await documentation; fix for Underscore templating #4351

merged 8 commits into from
Dec 20, 2016

Conversation

GabrielRatener
Copy link
Contributor

Added async function documentation as suggested by @GeoffreyBooth, and fixed a documentation generation bug.

@lydell
Copy link
Collaborator

lydell commented Nov 4, 2016

When documentation is updated, only index.html.js is usually updated. Compilation is done when releasing. This makes review easier.

@GabrielRatener
Copy link
Contributor Author

@lydell OK, how about now? l kept the Cakefile bug fix in this PR, but I can remove it if you think that belongs in a separate PR.

through the <code>yield</code> keyword. There's no <code>function*(){}</code>
CoffeeScript also supports
<a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function*">generator functions</a> and <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function">async functions</a>
through the <code>yield</code> and <code>await</code> keywords respectively. There's no <code>function*(){}</code> or <code>async function(){}</code>
nonsense &mdash; a generator in CoffeeScript is simply a function that yields.
Copy link
Collaborator

Choose a reason for hiding this comment

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

a generator in CoffeeScript is simply a function that yields, and an async function in CoffeeScript is simply a function that awaits. And then remove the Likewise line below.

@@ -184,10 +184,10 @@ task 'doc:site', 'watch and continually rebuild the documentation for the websit

do renderIndex = ->
codeSnippetCounter = 0
rendered = _.template fs.readFileSync(source, 'utf-8'),
render = _.template fs.readFileSync(source, 'utf-8')
Copy link
Collaborator

Choose a reason for hiding this comment

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

What did you fix here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The newer Underscore has a different templating API.

@GeoffreyBooth GeoffreyBooth changed the title Documentation updates Async/await documentation; fix for Underscore templating Nov 5, 2016
@GeoffreyBooth
Copy link
Collaborator

Do we want the example to be something real that people could run in their browser consoles? Like the first example in this comment? But it would have to be rewritten to not be generic . . . maybe change promisingOperation to flipCoin or somesuch, and probably add try/catch to catch the exception thrown by the rejected promise.

Whether we use that example or something else, because each example block has a Run button it would probably be best that the example would be able to run for users.

@GabrielRatener
Copy link
Contributor Author

@GeoffreyBooth That might be a good idea, but the sample won't run in most browsers today.

How about

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

window.countdown = (seconds) ->
  for i in [seconds..1]
    alert "#{i} second(s) to go..."
    await sleep(1000)  # wait one second
  alert "done!"

Then we could just run countdown(3)?

@lydell
Copy link
Collaborator

lydell commented Nov 5, 2016

This is good. I think I like the sleep example better. 👍

@GeoffreyBooth
Copy link
Collaborator

Yeah, this is a better example. We should change the alerts to console.logs, though, as we don't want the browser creating tons of pop-ups.

@lydell
Copy link
Collaborator

lydell commented Nov 5, 2016

We should change the alerts to console.logs, though

Meh. Other examples use alert (and none use console.log). I think this PR is fine for merging as-is.

@GeoffreyBooth
Copy link
Collaborator

The problem with alert is that it makes the example not do what it says it’s going to do. Say you do countdown(3), which triggers an alert saying “3 second(s) to go...”—if you let that dialog box sit for 10 seconds, then dismiss it, another second will go by before a new alert of “2 second(s) to go...”. Your 3-second countdown has taken far longer than 3 seconds.

I see your point about console.log though. How about this?

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

window.countdown = (seconds) ->
  document.body.innerHTML += '<aside id="countdown" 
    style="position: fixed; top: 50%; left: 0; right: 0;
    text-align: center; font-size: 75vh"></aside>'
  for i in [seconds..1]
    document.getElementById('countdown').innerHTML = i
    await sleep(1000) # wait one second
  document.body.removeChild document.getElementById('countdown') # done!

You can run it via the console in Chrome Canary:

var sleep;

sleep = function(ms) {
  return new Promise(function(resolve) {
    return window.setTimeout(resolve, ms);
  });
};

window.countdown = async function(seconds) {
  var i, j, ref;
  document.body.innerHTML += '<aside id="countdown" style="position: fixed; top: 50%; left: 0; right: 0; text-align: center; font-size: 75vh"></aside>';
  for (i = j = ref = seconds; ref <= 1 ? j <= 1 : j >= 1; i = ref <= 1 ? ++j : --j) {
    document.getElementById('countdown').innerHTML = i;
    await sleep(1000);
  }
  return document.body.removeChild(document.getElementById('countdown'));
};

countdown(3);

@GabrielRatener
Copy link
Contributor Author

@GeoffreyBooth I have the same issue, but your example seems a bit too complex. How about

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

window.countdown = (seconds) ->
  if not window.speechSynthesis?
    alert('speech API not supported in your browser')
    return

  for i in [seconds..1]
    utterance = new SpeechSynthesisUtterance("#{i}")
    speechSynthesis.speak(utterance)
    await sleep(1000)  # wait one second
  alert "done!"

?

@lydell
Copy link
Collaborator

lydell commented Nov 6, 2016

Good point about alert being blocking. Since support for speech synthesis is so good, I really like that example! How about this one (untested):

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

window.countdown = (seconds) ->
  for i in [seconds..1]
    if window.speechSynthesis?
      utterance = new SpeechSynthesisUtterance "#{i}"
      window.speechSynthesis.speak utterance
    console.log i
    await sleep 1000 # wait one second
  alert "Done! (Check the console!)"

@GeoffreyBooth
Copy link
Collaborator

Wow, I thought the speech synthesis thing was a joke. Yeah, that’s a pretty cool example.

Is it worth even checking for support? It’s supported in the one browser where async/await is supported (Chrome Canary). And we’re not checking for async/await support, though we could. But I gather from your dislike of the DOM element approach that you’re aiming for the most minimal example possible? How about this?

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

countdown = (seconds) ->
  for i in [seconds..1]
    window.speechSynthesis.cancel() # cancel any prior utterances
    window.speechSynthesis.speak new SpeechSynthesisUtterance "#{i}"
    await sleep 1000 # wait one second
  window.speechSynthesis.speak new SpeechSynthesisUtterance 'Blastoff!'

countdown(3)

No alert and no console.log, and we still show how something can happen after the last iteration of the loop.

@lydell
Copy link
Collaborator

lydell commented Nov 6, 2016

That’s also good. The only thing I worry about is if Firefox starts supporting await before speech synthesis.

@GabrielRatener
Copy link
Contributor Author

@GeoffreyBooth That works, but to simplify things I added

say = (text) ->
  window.speechSynthesis.cancel() # cancel any prior utterances
  window.speechSynthesis.speak new SpeechSynthesisUtterance text

@GeoffreyBooth
Copy link
Collaborator

Thanks @GabrielRatener, the current version looks great.

I’m happy with it as is, but do we want to add some kind of compatibility check? Speaking of Firefox, apparently a few days ago it was added to their nightly build: https://blog.nightly.mozilla.org/2016/11/01/async-await-support-in-firefox/ That page has a pretty cool example and demo, too.

I just checked in Firefox, and speech synthesis is already supported. caniuse.com says that a flag needs to be set, but it was enabled by default for me.

I’ve been trying to find a way to test for async/await support, and unfortunately the only ways to do so either involve loading an external file (like in the Mozilla demo) or using the dreaded eval:

try
  eval "(async function() {
    return 10 + (await Promise.resolve(10));
  })() instanceof Promise"
  asyncSupported = yes
catch error
  asyncSupported = no

I gather we probably don’t want to put an eval into any of our examples, so I guess it’s probably better to let this example be as it is without a compatibility check? Browsers that don’t support async/await will throw a SyntaxError that can’t be caught.

@GabrielRatener
Copy link
Contributor Author

GabrielRatener commented Nov 7, 2016

@GeoffreyBooth That seems really messy. Maybe I can just add a comment about browser compatibility for now. How about

sleep = (ms) ->
  new Promise (resolve) ->
    window.setTimeout resolve, ms

say = (text) ->
  window.speechSynthesis.cancel() # cancel any prior utterances
  window.speechSynthesis.speak new SpeechSynthesisUtterance text

# works only in browsers supporting ES async/await
countdown = (seconds) ->
  for i in [seconds..1]
    say "#{i}"
    await sleep 1000 # wait one second
  say "Blastoff!"

countdown(3)

?

@GeoffreyBooth
Copy link
Collaborator

@GabrielRatener Maybe put the comment at the top, and mention that the browser also needs to support speech synthesis?

Also I think you can just do say i rather than say "#{i}". At least in Chrome, window.speechSynthesis.speak(new SpeechSynthesisUtterance(1)) says “one.”

@GeoffreyBooth GeoffreyBooth changed the title Async/await documentation; fix for Underscore templating [CS2] Async/await documentation; fix for Underscore templating Nov 13, 2016
@GeoffreyBooth GeoffreyBooth merged commit dc25f46 into jashkenas:2 Dec 20, 2016
@GeoffreyBooth GeoffreyBooth modified the milestone: 2.0.0 Feb 9, 2017
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.

3 participants