Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

How to check for undefined members of object on legacy browsers? #5853

Closed
mxdvl opened this issue Aug 25, 2022 · 1 comment
Closed

How to check for undefined members of object on legacy browsers? #5853

mxdvl opened this issue Aug 25, 2022 · 1 comment
Labels
documentation Documentation needed or being created dotcom-rendering
Milestone

Comments

@mxdvl
Copy link
Contributor

mxdvl commented Aug 25, 2022

As raised by @bryophyta in #5849 (comment), there are several ways of checking whether an method exists as a member of an object. We also have an auto-fixing ESLint rule that prevents us from using optional chaining on non-nullish values.

For example, the window.performance.getEntriesByType method is defined in lib.dom.d.ts, so we need to circumvent this rule to prevent errors in production.

Which options do people prefer?

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- not all browsers have this method
const option1  = window.performance.getEntriesByType?.('measure') ?? [];

const option2  = 'getEntriesByType' in window.performance ? window.performance.getEntriesByType('measure') : [];

const option3  = typeof window.performance.getEntriesByType === 'function' in  ? window.performance.getEntriesByType('measure') : [];
@bryophyta
Copy link
Contributor

Thanks for this! If option1 is functionally equivalent to the others then I'd be inclined towards that? It looks like the issue here is essentially in the type definitions themselves, right? (I'm assuming that TS is basing its definitions here on a set of browsers that it's decided to support?) So being explicit that this is the source of the issue here seems like a good idea to me. But I don't have a super strong opinion on this :)

@cemms1 cemms1 added this to the Health milestone Oct 4, 2023
@cemms1 cemms1 removed the Health label Oct 4, 2023
@guardian guardian locked and limited conversation to collaborators Oct 11, 2023
@alinaboghiu alinaboghiu converted this issue into discussion #9097 Oct 11, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
documentation Documentation needed or being created dotcom-rendering
Projects
Archived in project
Development

No branches or pull requests

3 participants