-
Notifications
You must be signed in to change notification settings - Fork 3.4k
More use of globalThis to save on code size. NFC #25417
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice 👍
557511c
to
ee67765
Compare
case 0x1005 /* ALC_DEVICE_SPECIFIER */: | ||
if (typeof AudioContext != 'undefined' || | ||
typeof webkitAudioContext != 'undefined') { | ||
if (globalThis.AudioContext || globalThis.webkitAudioContext) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can probably remove webkitAudioContext?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do that as a followup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it looks like the unprefixed version is needed until Safari 14.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird...wasn't Safari/Webkit the first to implement WebAudio back in the day?
But I see MDN says
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safari 14.1 (Release date: 2021-04-26)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Safari iOS its 14.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safari < 14.5 still needs the webkit prefix. I authored this on another call site recently: #25356
Weird...wasn't Safari/Webkit the first to implement WebAudio back in the day?
I think what happened is that they went ahead to implement a very first version of the draft/spec, and then the spec got matured, and they took a long time to come back and unprefix their implementation. The webkitAudioContext()
and the .noteOn()
quirks stayed for a long time.
Also, make node detection consistent between file_packager output and emscripten output. Followup to emscripten-core#25417
Also, make node detection consistent between file_packager output and emscripten output. Followup to #25417
Followup to #25381