optimizely / javascript-sdk

JavaScript SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://www.optimizely.com/products/experiment/feature-experimentation/
Apache License 2.0
80 stars 82 forks source link

[BUG] queueMicrotask is not supported on rather recent TVs breaking clients #926

Closed ToxicBakery closed 5 months ago

ToxicBakery commented 6 months ago

Is there an existing issue for this?

SDK Version

5.3.0

Current Behavior

In 5.3.0, setTimeout was replaced with queueMicrotask. Unfortunately, this was only added in Chrome 71 which seems like forever ago unless you're a Samsung or WebOS TV and in that case your 2020 or older model TV is likely running a chromium version older than that. As such you are greeted with an exception.

In our specific case I do not have a quick way to get these users updated and the Samsung and LG review processes take literally months for releases to be approved and rolled out to devices. TLDR I can't quickly fix this on my end.

Expected Behavior

It would be ideal, in my opinion, if the SDK fell back to setTimeout in the case of queueMicrotask not being defined.

Steps To Reproduce

  1. Figure out how to run chromium 70 or older
  2. Try to use a project targeted 5.3.0 or more likely, following the SDK instructions which is not version locked https://unpkg.com/@optimizely/optimizely-sdk/dist/optimizely.browser.umd.min.js

SDK Type

Browser

Node Version

No response

Browsers impacted

WebOS/Tizen (Chromium)

Link

No response

Logs

Uncaught (in promise) ReferenceError: queueMicrotask is not defined
    at e.handleNewDatafile (optimizely.browser.umd.min.js:1:88903)
    at e.onDatafileManagerReadyFulfill (optimizely.browser.umd.min.js:1:88220)

Severity

Affecting users

Workaround/Solution

Add fallback support to avoid the issue, ie:

if (typeof queueMicrotask === "undefined") {
  setTimeout(async () => {
     ...
  }
} else {
  queueMicrotask(async () => {
     ...
  }
}

Recent Change

This is the commit that introduced the issue https://github.com/optimizely/javascript-sdk/commit/75a01486272acfdc396cc0f159258a8066b6af99

Conflicts

No response

ToxicBakery commented 6 months ago

To add to this and possibly changed my suggestion, the JS SDK documentation states an ES5 compliant browser is required. Given this I would instead recommend dropping use of queueMicrotask entirely as it is not ES5 compliant as far as I can tell. setTimeout also seems to not be part of the ES5 specification but since it has been essentially 100% adoption for over a decade, it seems more reasonable to use.

The JavaScript (Browser) SDK requires a modern web browser that is ES5-compliant.

https://docs.developers.optimizely.com/full-stack-experimentation/docs/install-sdk-javascript

mikechu-optimizely commented 6 months ago

Hey @ToxicBakery. This is a fantastic find and edge case we'd not looked at/considered. Thank you so much for the very clear report.

I think we'll need to enhance the code and our e2e suite to test on older Chrome instances.

I've created internal ticket FSSDK-10201 which will go into our planning cycle.

junaed-optimizely commented 5 months ago

Hey @ToxicBakery, A Patch release (5.3.3) is available now with the fix.