Skip to content

Conversation

@qm3ster
Copy link

@qm3ster qm3ster commented Aug 28, 2018

Initial pass

  • Also added retries to http, default set to 3, because I kept getting false negative tests due to bad internet connectivity.
  • Also added parcel. Browser tests seem to pass with it as well, but only if index.ts exports the class as es6 default, not cjs module that browserify needs, hence the src/parcel.ts file.
  • The typings in lib/ are still the old handcrafted ones, which is why definitions aren't set to output.
  • The only meaningful refactor is promise/error handling in index.js and parse.js, for example:

index.js:

$RefParser.prototype.bundle = function (path, schema, options, callback) { var me = this; var args = normalizeArgs(arguments); return this.resolve(args.path, args.schema, args.options) .then(function () { bundle(me, args.options); // If we put `maybe` on the outside, we can just return the value. return maybe(args.callback, Promise.resolve(me.schema)); }) .catch(function (err) { // If we put `maybe` on the outside, this becomes a noop rethrow function. return maybe(args.callback, Promise.reject(err)); }); };

became

 $RefParser.prototype.bundle = function (path, schema, options, callback) { var me = this; var args = normalize_args_1.normalizeArgs(arguments); return maybe(args.callback, this.resolve(args.path, args.schema, args.options).then(function () { bundle_1.default(me, args.options); return me.schema; })); };

and likewise for the rest of the public methods.

parse.js

function readFile (file, options) { return new Promise(function (resolve, reject) { debug('Reading %s', file.url); // Find the resolvers that can read this file var resolvers = plugins.all(options.resolve); resolvers = plugins.filter(resolvers, 'canRead', file); // Run the resolvers, in order, until one of them succeeds plugins.sort(resolvers); plugins.run(resolvers, 'read', file) .then(resolve, onError); function onError (err) { // Throw the original error, if it's one of our own (user-friendly) errors. // Otherwise, throw a generic, friendly error. if (err && !(err instanceof SyntaxError)) { reject(err); } else { reject(ono.syntax('Unable to resolve $ref pointer "%s"', file.url)); } } }); }

became

function readFile (file, options) { debug_1.default('Reading %s', file.url); // Find the resolvers that can read this file var resolvers = plugins_1.all(options.resolve); resolvers = plugins_1.filter(resolvers, 'canRead', file); // Run the resolvers, in order, until one of them succeeds resolvers = plugins_1.sort(resolvers); return plugins_1.run(resolvers, 'read', file).catch(function (err) { // Throw the original error, if it's one of our own (user-friendly) errors. // Otherwise, throw a generic, friendly error. if (err && !(err instanceof SyntaxError)) { throw err; } else { throw ono.syntax('Unable to resolve $ref pointer "%s"', file.url); } }); }

Only catching the error to decorate it, not re-resolving the success.
Becomes a linear function instead of using callbacks.

Initial pass Also added retries to http, default at 3, because I kept getting false negative tests due to bad internet connectivity Also added `parcel`. Browser tests seem to pass with it as well, but only if `index.ts` exports the class as es6 default, not cjs module, like browserify needs, hence the `src/parcel.ts` file. The typings in `lib/` are still the old handcrafted ones, which is why definitions aren't being output
@qm3ster
Copy link
Author

qm3ster commented Aug 28, 2018

Also, I can swear I made this PR 4 hours ago, when I uploaded the commit, but now I can't find any trace of that. It wasn't a PR of my branch to my master or anything. Maybe I'm dying.

@qm3ster qm3ster mentioned this pull request Aug 28, 2018
@qm3ster qm3ster changed the title Convert to TypeScript Convert to TypeScript 💯 Aug 28, 2018
@qm3ster qm3ster changed the title Convert to TypeScript 💯 Convert to TypeScript 💯 Aug 28, 2018
@qm3ster
Copy link
Author

qm3ster commented Aug 29, 2018

Travis failed because of a wrong typedef in JS-DevTools/ono#5
I had it corrected locally so didn't get this error.

@G-Rath
Copy link
Contributor

G-Rath commented Jan 17, 2021

@APIDevTools I'd be keen to pick this up if it's something you're interested in :)

@philsturgeon
Copy link
Member

Thanks @qm3ster and @G-Rath for your interest! There is a new library coming along which is going to replace this one, and it's still private, but so future people have some way of knowing: https://github.com/APIDevTools/json-schema-reader

@G-Rath
Copy link
Contributor

G-Rath commented Jan 18, 2021

@philsturgeon sounds exciting - do you know if its more performant than this one?

I found that this library was choking on dereferencing a large schema, which I was planning to make an issue for at some point.

@philsturgeon
Copy link
Member

@G-Rath the FHIR JSON Schema will melt the strongest CPU so I'm not surprised. A performance improvement was just made today in 9.0.7 so see if that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants