digitalbazaar / http-client

An opinionated, isomorphic HTTP client.
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

http-client loading error #27

Closed johnwen84 closed 2 years ago

johnwen84 commented 2 years ago

Hello, I upgraded to the latest jsonld (6.0.0) and jsonld-signatures (10.0.0) packages, and am getting this error in http-client (3.2.0) when trying to run my app. Please let me know how I can get around it. @davidlehn I'm trying this after receiving the github notification of availability of jsonld 10.0.0 with the fix I have been waiting for. Thanks John

/Users/johnwen/package-test/JsonLd/node_modules/@digitalbazaar/http-client/dist/cjs/index.cjs:27 if(options?.fetch && !options.fetch._httpClientCustomFetch) { ^

SyntaxError: Unexpected token '.' at wrapSafe (internal/modules/cjs/loader.js:915:16) at Module._compile (internal/modules/cjs/loader.js:963:27) at Module._extensions..js (internal/modules/cjs/loader.js:1027:10) at Object.require.extensions. [as .js] (/Users/johnwen/package-test/JsonLd/node_modules/ts-node/src/index.ts:485:14) at Module.load (internal/modules/cjs/loader.js:863:32) at Function.Module._load (internal/modules/cjs/loader.js:708:14) at Module.require (internal/modules/cjs/loader.js:887:19) at require (internal/modules/cjs/helpers.js:74:18) at Object. (/Users/johnwen/package-test/JsonLd/node_modules/jsonld/lib/documentLoaders/node.js:12:22) at Module._compile (internal/modules/cjs/loader.js:999:30)

mattcollier commented 2 years ago

Hello, it appears that your environment is taking issue with the optional chaining syntax being used here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining

I would refer you to the browser compatibility matrix on that same page: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining#browser_compatibility

It looks like you'll need Node.js 14+ if you're running Node.js.

johnwen84 commented 2 years ago

@mattcollier Thanks for the quick response. Hmmm we are restricted to NodeJS 12 because of another package we use in our app. Optional chaining is cool, but I guess it's a bit too advanced for us in our current situation. Would it be possible to roll it back or somehow get around it? Thanks

dlongley commented 2 years ago

@johnwen84, No, unfortunately -- the underlying ky library (and its security fixes) required a move to node 14.

I will note that Node 12 is EOL (end of life). It stopped receiving active support over a year and a half ago and stopped receiving security support over a month ago.

https://nodejs.org/en/about/releases/ https://endoflife.date/nodejs

I would recommend updating the other package instead.

mattcollier commented 2 years ago

I will note also, that dropping support for Node 12 is one of the breaking changes in http-client@3.

https://github.com/digitalbazaar/http-client/blob/main/CHANGELOG.md#removed-1

johnwen84 commented 2 years ago

@mattcollier @davidlehn thanks for the info, I'll discuss with the team and see what we can do.

davidlehn commented 2 years ago

All the above being said, the core of jsonld.js and jsonld-signatures should still work with Node.js 12. The issue is when jsonld.js loads the default node document loader which uses @digitalbazaar/http-client. There may be some module resolver tricks that could avoid that being loaded. But that's only useful if you have your own custom document loader. I suspect in the future we'll refactor things to be more flexible in what document loaders are imported by default, if any. But that's not much help right now. Upgrading Node really is the best option if you can do it. These projects and others will continue to use newer features.

johnwen84 commented 2 years ago

Thanks @davidlehn. That gives us at least an option.

johnwen84 commented 2 years ago

@davidlehn @mattcollier just to let you know that I upgraded nodeJS to 16 but still getting the error. I confirmed that optional chaining is now supported. I tested in my main code and it works fine there. But somehow it's not recognized in http-client as an external module. I'm still working on this, but please let me know if you have any suggestions. We use TypeScript BTW.

mattcollier commented 2 years ago

I just ran the internal test suite on node v16.15.0, and all tests are passing. Can you try same?

https://github.com/digitalbazaar/http-client/blob/586ee0e8044a4139047e27ec89236155a46f0087/package.json#L21-L23

johnwen84 commented 2 years ago

@mattcollier I'm getting this error running the test. I guess it might be something on my machine. I'll clean it up and try again...

johns-mbp:http-client johnwen$ npm run test

@digitalbazaar/http-client@3.2.1-0 test npm run test-node && npm run test-node-cjs

@digitalbazaar/http-client@3.2.1-0 test-node cross-env NODE_ENV=test mocha --preserve-symlinks -t 30000 -A -R ${REPORTER:-spec} --require tests/test-mocha.js tests/*.spec.js

SyntaxError[ @/Users/johnwen/package-test/http-client/tests/10-client-api.spec.js ]: Unexpected token '.' at Loader.moduleStrategy (internal/modules/esm/translators.js:140:18) johns-mbp:http-client johnwen$

mattcollier commented 2 years ago

OK, yes, you should be able to see that the test suite is green on the github CI test: https://github.com/digitalbazaar/http-client/runs/6515471107?check_suite_focus=true

johnwen84 commented 2 years ago

@mattcollier it was a problem on my machine. I have both NodeJS 12 and 16, when I switch from 12 to 16, npm still uses 12 for some reason. I have it all fixed. I'm still having some strange issues in the main app, but I can now run http-client testcase and some smaller test apps with the latest jsonld and jsonld-signatures. Thanks for all the help!

dlongley commented 2 years ago

@johnwen84 glad it all got sorted! :) Closing this.