search

51Degrees / Device-Detection

THE Fastest and most Accurate device detection for C / PHP / Perl / Python and Node.js - professionally maintained device data
https://51degrees.com/device-detection
Other
112 stars 46 forks source link

Node version must be >= 6.5 and <= 12. #53

Closed jeanlescure closed 3 years ago

jeanlescure commented 3 years ago

Expected Behavior

To be able to install fiftyonedegreescore in a project based on Node.js 14, the current Long Term Support (LTS) version.

Current Behavior

If your active Node install is 14.x and either run yarn/npm install on a repo with the latest version of fiftyonedegreescore as a dependency or if you try to add the latest version of fiftyonedegreescore to a new repo, the install fails with the following error:

/clickster-admin/node_modules/fiftyonedegreescore/versionCheck.js:28
        throw "Node version must be >= 6.5 and <= 12.";
        ^
Node version must be >= 6.5 and <= 12.
jeanlescure commented 3 years ago

Tried forking this repo and simply changing the version check to allow Node v14, but got multiple errors from node-gyp, most of them stemming from the following types of errors:

3673 |   V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
      |                                     ^~~
/home/***/.cache/node-gyp/14.15.0/include/node/v8.h:3673:37: note:   candidate expects 3 arguments, 2 provided

It seems SWIG generates calls to Set with 2 arguments where there should be 3.

I've opened an issue on swig/swig (#1898) for them to hopefully take a look at this ASAP.

My understanding of SWIG and the way 51Degrees generates the bindings for Node is very limited, so I would encourage your team to provide more context for SWIG's team on the aforementioned issue (or even a PR/fix if it's within the realm of possibilities).

Steve51D commented 3 years ago

Hi @jeanlescure,

Apologies for the slow response. For the most part, we are no longer updating these older versions of the device detection API.

Version 4 of the API is tested against Node v14.

jeanlescure commented 3 years ago

Hi @Steve51D , version 4 is not available on npm: https://www.npmjs.com/package/fiftyonedegreescore

Latest fiftyonedegreescore package on npm is standing at version 3.2.24

Steve51D commented 3 years ago

It has a different package name: https://www.npmjs.com/package/fiftyone.devicedetection

Sorry for the confusion!

jeanlescure commented 3 years ago

@Steve51D , it's relevant to mention that we hold in high regard the device detection package you and your team maintain; it drives key features that the company I work for offers to our user-base.

With that said, the Enterprise license, that the company I work for has been paying for 5 years, only allows us to download V34 datafiles.

So, it seems from what you are telling me, this year my company paid for a license for a product you are "are no longer updating", and from what I gather from my attempts to solve this on the 51Degrees website, with no option to upgrade to the latest unless we buy another license.

Something that is also not documented in the migration guide:

https://51degrees.com/documentation/4.1/_device_detection__migration_guides_51_degrees_v3.html

I tried calling support but our work hours do not overlap.

We would very much appreciate it if you could give us some light on how to solve this situation.

Steve51D commented 3 years ago

Hi @jeanlescure,

I've passed your details over to our customer success manager, who will be in contact directly.

tungntpham commented 3 years ago

Hi @jeanlescure,

We have just released the 4.2.1 version of Node with support for Node 14. We have tried and tested this version of SWIG and it worked fine for us. You can also see my comment on the PR.

I have let our customer success manager know, and he will be in contact with you directly. Meanwhile, if you want to try, please go ahead and download it from npmjs.

Kind Regards, Tung.