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 userAgent missing #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

albert-carreras
Copy link

Self explanatory!

It would be useful for not breaking frameworks like react-native, that don't have userAgent in their navigator

@styfle
Copy link

styfle commented Oct 20, 2023

Looks like this will fix issue #28 👍

@flexdinesh would you consider merging?

@flexdinesh
Copy link
Owner

This project is so old I can't even run it in my local anymore. I might have to upgrade the build setup and get rid of most of them because we don't need 'em anymore in 2023. Once I upgrade I will release this change as part of a new major.

In the meanwhile, are any of you able to run the branch in your local machine? I'm happy with the change but quite hesitant to merge with the optional chaining operator ?. This package is consumed downstream in really old versions of node and tv browsers and optional chaining might break those versions if it's not compiled with the current build setup (which I couldn't check now). If you could patch the change and replace the optional chaining with object key checks I'd be happy to merge and release a patch.

@jeff-mccoy
Copy link

We are also hitting this with Node 21 as this is a downstream dep of Quicktype-core. I think the optional operator is probably fine since this is 1. a new version bump and 2. been supported since Node 14. That being said, you could also add an engine param to the package.json to warn users trying to use it on older version of Node:

  "engines": {
    "node": ">=140.0"
  },

@flexdinesh
Copy link
Owner

Changes shipped in 3.0.0-pre.0

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 this pull request may close these issues.

None yet

4 participants