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 the usage of hasOwn #150

Closed
sjaq opened this issue Feb 5, 2024 · 1 comment · Fixed by #151
Closed

Consider the usage of hasOwn #150

sjaq opened this issue Feb 5, 2024 · 1 comment · Fixed by #151

Comments

@sjaq
Copy link
Contributor

sjaq commented Feb 5, 2024

  • match-sorter version: 6.3.3
  • node version: 20.4.0

Relevant code or config

  if (typeof key === 'function') {
    value = key(item)
  } else if (item == null) {
    value = null
  } else if (Object.hasOwn(item, key)) {
    value = (item as IndexableByString)[key]
  } else if (key.includes('.')) {
    // eslint-disable-next-line @typescript-eslint/no-unsafe-call
    return getNestedValues<ItemType>(key, item)
  } else {
    value = null
  }

What you did: Used matchSorter in my project and tested it in Safari 13.1
What happened: TypeError: Object.hasOwn is not a function

Screenshot 2024-02-05 at 16 45 32

Problem description:
For our music industry specific project we have to support Safari 13.1 for a little longer as a lot of music producers are stuck at older macOS versions. match-sorter relies on the Object.hasOwn function that is not available in an older browser like Safari 13.

Suggested solution:
Switch back to using the previous hasOwnProperty setup.

I understand using this newer API simplifies the code, but it does introduce a need for us to add polyfills for older browsers or consider our usage of match-sorter. In this case specifically the change is so small I wanted to raise an issue as I totally get you have to draw the line somewhere regarding browser support. Just wanted to make sure the change was intentional as I didn't see any reference of a breaking change in the commit history or release log and it seems to have been introduced in an unrelated update #148

@kentcdodds
Copy link
Owner

I'm happy to accept a PR to switch this to hasOwnProperty :)

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

Successfully merging a pull request may close this issue.

2 participants