feat: make the lib isomorphic

alanpoulain commented 1 year ago

Fixes #275.

In order to make this lib isomorphic (usable both in the browser and in Node.js), these changes have been made:

do not use process global variable directly
use fetch instead of http (Fetch API is available in Node.js as experimental and if not, can be polyfilled easily).
do not use url (use URL API instead).

philsturgeon commented 1 year ago

Thank you so much @alanpoulain!

Linter has noticed a few things:


/home/runner/work/json-schema-ref-parser/json-schema-ref-parser/lib/resolvers/http.js
Error:     3:7   error  'http' is assigned a value but never used   no-unused-vars
Error:     4:7   error  'https' is assigned a value but never used  no-unused-vars
Error:   148:13  error  Strings must use doublequote                quotes
Error:   150:48  error  Strings must use doublequote                quotes
Error:   150:60  error  Strings must use doublequote                quotes

/home/runner/work/json-schema-ref-parser/json-schema-ref-parser/lib/util/url.js
Warning:   25:18  warning  'url' is already declared in the upper scope  no-shadow

I would just fix those myself but that last one might need a closer eye to make sure there's nothing funny going on.

philsturgeon commented 1 year ago

I had a go at fixing the linting issues myself in #282 but there is a test failure too.

  148 passing (525ms)
  1 failing

  1) HTTP options
       http.headers
         should override default HTTP headers:
     ResolverError: Error reading file "https://httpbin.org/headers"
      at onError (lib/parse.js:32:232)

Can you make sure npm run test:node is working before you push?

Thank you!

alanpoulain commented 1 year ago

Sorry I forgot to run the linter, it should not be an issue anymore!

For the test failure, I was testing with Node.js 18, hence I didn't have the failure. It should be fixed now: I added the node-fetch polyfill in the tests. I've also added node-abort-controller for Node.js 14 tests.

I have updated the CI as well.

I've also added a mention to use a polyfill in the README.

philsturgeon commented 1 year ago

Thank you for such a speedy fix!

There's one more issue popping up on Node v14 and I have seen it in a few places, not sure why...

Run npm run coverage:node

> @apidevtools/json-schema-ref-parser@0.0.0-dev coverage:node D:\a\json-schema-ref-parser\json-schema-ref-parser
> nyc node_modules/mocha/bin/mocha

  File names with special characters
    1) should parse successfully

  0 passing (47ms)
  1 failing

  1) File names with special characters
       should parse successfully:
     TypeError [ERR_INVALID_URL]: Invalid URL: specs/__(%7B%5B%20%25%20&%20$%20%23%20@%20%60%20~%20,)%7D%5D__/__(%7B%5B%20%25%20&%20$%20%23%20@%20%60%20~%20,)%7D%5D__.yaml
      at new NodeError (internal/errors.js:322:7)
      at onParseError (internal/url.js:270:9)
      at new URL (internal/url.js:346:5)
      at Object.resolve (lib\util\url.js:8:137)
      at $RefParser.parse (lib\index.js:46:39)
      at Context.<anonymous> (test\specs\__({[ % & $ # @ ` ~ ,)}]__\special-characters.spec.js:13:33)
      at processImmediate (internal/timers.js:464:21)

Any chance you can smash that one too?

alanpoulain commented 1 year ago

That's weird, I cannot reproduce it on my machine, it seems a Windows only issue. It doesn't seem related to the changes of this PR too. I won't have time to look at it before a few days.

philsturgeon commented 1 year ago

@alanpoulain thank you, I'll make a fresh issue for it. Merged!

lucy-loo commented 1 year ago

@philsturgeon can you publish a new version? 🙏

APIDevTools / json-schema-ref-parser

feat: make the lib isomorphic #280