search

flexdinesh / browser-or-node

Check where your code is running. In the browser or in node.js environment.
MIT License
245 stars 20 forks source link

consider userAgent missing #25

Open albert-carreras opened 1 year ago

albert-carreras commented 1 year ago

Self explanatory!

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

styfle commented 1 year ago

Looks like this will fix issue #28 👍

@flexdinesh would you consider merging?

flexdinesh commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Changes shipped in 3.0.0-pre.0