decodeAudioData w/promise

[sidewayss]

A new example page for decodeAudioData() that uses promises, fetch() and async/await. Also cleans up the user interface a tiny bit. This example goes hand-in-hand with edits I have made to the MDN docs page for decodeAudioData() in this PR: https://github.com/mdn/content/pull/26839 This is not intended to replace the current decode-audio-data example. It is intended to complement it.

[github-actions[bot]]

It looks like this is your first pull request. 🎉 Thank you for your contribution! One of the project maintainers will triage and assign the pull request for review. We appreciate your patience. To safeguard the health of the project, please take a moment to read our code of conduct.

[wbamberg]

@sidewayss , if you set the GitHub Pages branch in your fork of this repo to the dAD-Promise branch, then I'll be able to try out this example live at something like https://sidewayss.github.io/webaudio-examples/decode-audio-promise/. That would be super-helpful for me :).

[sidewayss]

I'm not sure exactly what you mean. My repo only has a main and dAD-Promise branch. Do you want me to create a new branch of dAD-Promise named GitHub Pages?

[sidewayss]

If you like I can host it on my personal domain in a sub-directory.

[sidewayss]

I just put it on my personal domain here: https://sidewayss.com/dAD/ I copied the files from the cloned repo on my disk, not the copy that's in my localhost for testing, if that makes you feel more confident that it's the same as what's in the PR.

[wbamberg]

I'm not sure exactly what you mean. My repo only has a main and dAD-Promise branch. Do you want me to create a new branch of dAD-Promise named GitHub Pages?

What I meant was: in the settings for your fork of this repo, you can tell GitHub to build Pages from a specific branch: https://docs.github.com/en/pages/getting-started-with-github-pages/configuring-a-publishing-source-for-your-github-pages-site#publishing-from-a-branch. If you set that to the branch for this PR, then I can essentially see a deploy preview for this branch.

For instance if you visit https://wbamberg.github.io/dom-examples/web-crypto/derive-key/index.html you'll see the page deployed from https://github.com/mdn/dom-examples/pull/212.

But what you have done here works fine too.

[sidewayss]

OK, there it is at: https://sidewayss.github.io/webaudio-examples/decode-audio-promise/ - just as you predicted. This way you know it's exactly what's in the PR. Thanks for cluing me in to the github feature.

[sidewayss]

It only fetches the audio file once per session instead of every time you click Play. It's a more current coding style in terms of the elements and events (I never use querySelector as it performs so poorly, though in an example like this it doesn't matter). It has a simpler approach to enabling controls. It prevents bogus loop parameters, setting a loop duration minimum of 1 second. But if you want it to be a clean comparison I can make it so that it's a minimum of changes from the current XHR example. I would like to at keep least the change of fetching the file only once per session, as that is a better approach and relates directly to this function.

[sidewayss]

Also note: The docs page says that that the example uses createBufferSource() to create the audioBufferSourceNode, and the example in the docs page does this. But the example page does not. It uses new AudioBufferSourceNode. That is another change I would like to keep.

[sidewayss]

I have created a version with minimal changes from the XHR example (except the two I mentioned most recently), and I have pushed it to my branch here on github, as you can see by this page. The Github Pages page is updated too. Good thing you all squash commits. Let me know if this is more to your liking. It's definitely not as inviting a page with all the widgets disabled when you open it :-)

The one change not mentioned above is wrapping the fetchAudio call and Promise.then() in a pageLoad() function. It just looked weird hanging out there in the global script space, and it's certainly not bad practice.

[sidewayss]

I understand the desire for minimal differences between the examples, so I have no problem with you rejecting my first pass here. The original example works as an example of using decodeAudioData, but it's a bummer of an interface, especially the way it disables controls. Some of the code is also anathema to me, like querySelector() and using classes instead of ids the way it does. That's why I "cleaned it up" with my first pass, fwiw.

I have committed or responded to your suggested changes. As for editing the existing XHR example, I can probably do that. You just want to make them identical except for the Promise vs XHR, right? I might take the liberty to then swap all the querySelector calls for getElementById and change the class attributes to id attributes for the UI elements in both examples, just to assuage my cleanup tendencies. My first pass iterated over the elements using child/sibling relationships, which is even better, but I can live with getElementById :-)

[wbamberg]

I understand the desire for minimal differences between the examples, so I have no problem with you rejecting my first pass here. The original example works as an example of using decodeAudioData, but it's a bummer of an interface, especially the way it disables controls. Some of the code is also anathema to me, like querySelector() and using classes instead of ids the way it does. That's why I "cleaned it up" with my first pass, fwiw.

I have committed or responded to your suggested changes. As for editing the existing XHR example, I can probably do that. You just want to make them identical except for the Promise vs XHR, right?

Yes.

I might take the liberty to then swap all the querySelector calls for getElementById and change the class attributes to id attributes for the UI elements in both examples, just to assuage my cleanup tendencies. My first pass iterated over the elements using child/sibling relationships, which is even better, but I can live with getElementById :-)

OK, if you wish.

[sidewayss]

It just occurred to me that changing the original example means changing the text of the example in the docs page too. They are currently misaligned anyway. IMO the example text in the docs page should be much shorter and focused on decodeAudioData, and that's how I did the update Promise example in the docs. What do you think? Should I align the two docs examples too? And should I do it in the same pending PR I have open?

[sidewayss]

I have updated the original example locally and tested it. I made the two examples look as similar as possible, plus updating the ancient-looking calls to setAttribute and removeAttribute with direct property settings. Do you want me to do both examples in this one PR or do you want a separate one for the original example? For now I have the updated original in a new branch here, so you can review my changes. But I can incorporate those changes into this branch if you want it all in one PR. Once you approve of these changes, I'll make the corresponding changes in this Promise-based example and the two will align nicely. After discussing it at such length, of course the XHR example sets the global buffer inside the callback because that's the only way. So once the XHR example is approved I'll change this example to put it in the .then() as you suggested and make the two the same.

[sidewayss]

fyi - I just realized that the original XHR example has a bug. See here: https://github.com/mdn/webaudio-examples/blob/3eb15bde7bfbab77ba8ec7abf38e00a8ae292bdc/decode-audio-data/index.html#L97 Error objects don't have a .error property (the docs page uses err.err, which is even more wrong). It is fixed in my new branch here: https://github.com/sidewayss/webaudio-examples/blob/0eb16c0d169d337fff1a9361fea7b55ef6a4b88d/decode-audio-data/index.html#L94

[sidewayss]

Should I be running Prettier on these examples? Seems appropriate, but I'll leave it up to you.

[wbamberg]

Should I be running Prettier on these examples? Seems appropriate, but I'll leave it up to you.

Yes, we should, if only because the copy in the MDN pages will be auto-Prettified, so if we don't they will be different. I just pushed that change, hope you don't mind.

[wbamberg]

Oh, I just realised, you have to sign your commits for this repo (and actually all MDN repos). Are you set up to do that? I think it might be complicated to sign existing commits in this PR (because it effectively changes history), maybe it would be simpler to close this PR and open a new one? Sorry for the extra work.

[sidewayss]

Yea, I had SSH set up here on github.com several years ago, but I had to reinstall Windows in 2021 and it looks like that erased the local keys and thus effectively the whole deal. I can't think of any other reason for the disappearance. I have been absent from computing since the some time before the reinstall, thus absent from github too. I only recently realized that my SSH key was gone and didn't do anything about it because it wasn't pressing to do so.

I'll set up a new SSH key and see about a new PR or signing all these commits but probably not until the end of the day today. Here are some responses to your previous comments today:

Yes, we should, if only because the copy in the MDN pages will be auto-Prettified, so if we don't they will be different. I just pushed that change, hope you don't mind.

No problem. I'll try to remember to run Prettier before any future commits.

👍 this looks and works great, thank you! Let's merge it and then update the other example in a new PR, then update the MDN pages.

I'll open a new PR for the "new original" example and prettify the code.

[sidewayss]

@wbamberg - I don't have an email address for you, so I'm commenting here. I have made two passes at this comment because I was using the output from console.log() to test things, and it displays empty array elements as undefined, which they are not. I have got it straight now:

I have found some very confusing information in the MDN docs for Array.prototype.map(). JavaScript array functions handle empty elements in a few different ways, and it's not easy to sort them out if you're learning on the fly, as I have been today.

[0, , 2].map(x => x) returns [0, , 2] [0, , 2].filter(x => x) returns [0, 2]

map() iterates over the empty elements but does not pass them to the callback. The map() docs say this, but they could be much clearer that map() leaves the empty elements intact in the returned array, and so could the examples. I have had to test various Array.prototype functions today to be clear as to exactly what they do and don't do with empty elements. map() is especially confusing relative to filter() which literally does not iterate over the empty elements.

The MDN docs for Sparse Arrays are also misleading and have a confusing example for map(): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Indexed_collections#sparse_arrays At least the example should be changed to show an actual array with empty elements instead of the <2 empty items> designation, which is confusing.

Is this something that I should try to clarify with a PR for these docs pages? Or am I just ignorant and not reading the docs slowly and carefully enough? I don't think I should have to test these functions on jsfiddle or inside the code editor in an MDN docs page to figure out what they actually do. It's a confusing topic to begin with, the way javascript treats empty array elements. They're not undefined, but there is no constant value you can compare them to. Then you have different functions that treat them differently. It's confusing to begin with.

[wbamberg]

I think if you find an issue in MDN it is better to file it: you're much more likely to get eyes on it.

@wbamberg - I don't have an email address for you, so I'm commenting here. Today, coincidentally, I have found some very misleading information in the MDN docs for Array.prototype.map(). JavaScript array functions handle empty elements in a few different ways, and it's not easy to sort them out if you're learning on the fly, as I have been today. Turns out the map() docs are just plain wrong about how it handles empty elements, at least based on my tests in Chrome and Firefox: map() does not ignore empty elements the way filter() does.

[0,,2].map(x => x) returns [0,undefined,2] [0,,2].filter(x => x) returns [0,2]

I think both map and filter do not call the callback function for empty slots:

const a = [1,,2];

a.filter(x => {
  console.log("called");
  return x===undefined;
}); 
// called
// called
// Array [] 

a.map(x => {
  console.log("called");
  return x*2;
}); 
// called
// called
// Array(3) [ 2, <1 empty slot>, 4 ]

But while map() doesn't ignore empty elements, there is no way to test for them, for example:

[0,,2].map(x => x === undefined ? 0 : x) returns [0,undefined,2]

So map() puts undefined in every slot that is empty in the source array, regardless of the callback function. It's the worst of both worlds. Hard to believe that the way it's implemented is correct, but that's how it is.

No, map does not call the callback function for empty slots, but returns undefined when you try to access them. This is discussed in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Indexed_collections#sparse_arrays.

The MDN docs for Sparse Arrays are also misleading and have a confusing example for map() (what is <2 empty items>?): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Indexed_collections#sparse_arrays

MDN is not alone in this. Everywhere I look it's the same. Am I missing something? Are the docs everywhere just wrong? Did Google and Firefox implement map() incorrectly? I wanted to verify that I'm not nuts before I think about submitting a PR for these docs.

Can you be really specific about the error you see in the docs?

[sidewayss]

To quote the Indexed_collections#sparse_arrays we both linked to: But in others (most notably array iteration methods), empty slots are skipped. const mapped = arr.map((i) => i + 1); // [ 2, 3, <2 empty items>, 6 ]

a) That docs page does not say that map() "returns undefined", as you say, and map() doesn't do any returning of undefined for empty slots. It retains the empty slots intact, no change.

b) Saying that "empty slots are skipped", as that page does state, is misleading relative to map(). The callback skips them, but map() returns them intact, no change. They are not skipped entirely as with .forEach(), filter(), some(), every(), or Object.keys(), which do not include them in the results regardless of any callback. Those functions don't iterate over the empty elements at all because they use property enumeration (or at least that's how I assume they do it). map() iterates over the empty elements and copies them to the returned array, unlike all the other functions in that example except the spread operator, which does both: it acts like map() in let a = [...b] and it uses property enumeration in let o = {...b};. IMO that should be clarified for map(), and the whole thing could be clearer with a few more sentences of explanatory text. Right now there are only 5 sentences, and one of those is "for more information, go to..." . (also every() is missing from those examples, I think it's the only one missing)

c) IMO that line from the example should read [ 2, 3, , , 6 ] or [ 2, 3, <empty>, <empty>, 6 ] instead of [ 2, 3, <2 empty items>, 6 ]

It would be clearer. The <N empty items> comments are in previous examples, but not the examples for iteration that "skips" empty elements. IMO preserving all the commas in the literal representation of the array is important to reading it clearly. Everyone knows what a literal empty array element looks like, there's no need to abstract it to N empty items.

As for the map() page itself, a sentence or two clarifying these same points would be nice. map() and the spread operator are the outliers of this group.

So I was having trouble sorting out the details of this earlier, and I still had to test the spread operator just now to make sure I understood exactly what it does with empty elements. I was just trying to find a clean way to get the indexes of empty and undefined elements in an array. Turns out the best way is with an old-fashioned for (i = 0; i < l; i++) loop, which is hardly what I expected. My initial level of alarm might well have been too high. There are no grave errors in the docs, but they are far from clear and could use some additional explanatory detail.

[sidewayss]

OK, now I think I have a clear understanding of what map() does and a clear way of expressing it verbally:

map() sets the length of the new array to the length of the source array prior to iterating over only the non-empty elements to transfer values into the new array.

That's how it can not iterate over the empty elements and still effectively preserve them in the returned array. It makes sense to allocate the new array fully prior to copying if the lengths are to be the same.

Adding that clarification to the map() docs regarding sparse arrays and the Sparse Arrays docs regarding map() would be useful and concise. The Sparse Array docs could use more descriptive text and should include the other Array functions that don't handle empty elements, such as the indexOf functions and every().

How's that?

[wbamberg]

@sidewayss , would you mind filing a separate issue for this? You'd probably get better feedback and it would be easier to keep this one on track.

[sidewayss]

Well, that was an adventure. Creating and installing a GPG key for github.com using Windows is not quite the way the instructions here describe. Thank goodness for stackoverflow and a couple of other sites. I created a new PR with my 1 commit signed here: #105. I created a new PR for the updated XHR example here: #106 Enjoy!

[sidewayss]

@sidewayss , would you mind filing a separate issue for this? You'd probably get better feedback and it would be easier to keep this one on track.

Yes, I will do that. Sorry for the bother.