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

Consider typeof in feature detection #520

Open
Krinkle opened this issue Mar 28, 2022 · 1 comment
Open

Consider typeof in feature detection #520

Krinkle opened this issue Mar 28, 2022 · 1 comment

Comments

@Krinkle
Copy link

Krinkle commented Mar 28, 2022

The addition of feature detection in #327 is great. I think it would make a great addition to check for typeof.

if (fetch) {
    fetch()
}

if (localStorage); // ReferenceError
if (window.localStorage); // DOMException: Storage not allowed

This is generally unsafe as this would cause a ReferenceError if fetch did not exist. Instead, the safe and environment-agnostic way to check support for a built-in API is through typeof:

if (typeof fetch !== 'undefined') {
  fetch();
}
if (typeof fetch === 'function') {
  fetch();
}
if (typeof document !== 'undefined' && document.getElementById('qunit')) {
  init(document.getElementById('qunit-header'));
}

There is support for a different safe-ish mechanism already, namely window.fetch however I generally avoid this because 1) This can cause an exception if the property has a getter that denies access such as window.localStorage in some browser's private mode, and 2) is slightly slower, and 3) relies on there being a window global which doesn't work for isomorphic code that is agnostic of browser version and JS engine (e.g. also in Node.js, SpiderMonkey, especially versions prior to globalThis).

The typeof approach always works and seems to be common for this purpose.

I would recommend checking for either !== 'undefined' or === with anything other than 'undefined' (e.g. "object", "number", someVariable). I've not worked much with ESTree before, but I'd like to learn and would be interested in contributing a patch.

@NikolayFrantsev
Copy link

I fully agree with that. Current implementation of feature detection is harmful and there even no option to disable it. For example following code is considered valid even if browser has no fetch support:

const url = tryToSomeUrl();
if (url) {
  fetch(url);
}

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

No branches or pull requests

2 participants