Skip to content

api suggestion, possible performance gains #93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
andykais opened this issue May 17, 2019 · 2 comments
Open

api suggestion, possible performance gains #93

andykais opened this issue May 17, 2019 · 2 comments

Comments

@andykais
Copy link
Contributor

So currently, running the library works like this:

const { JSONPath } = require('jsonpath-plus')

const query = '<some query>'
const runQuery = (object) => {
  return JSONPath({ path: query, json: object })
}

if I run that query multiple times, there are a bunch of initialization steps that it will repeat that it does not need to (class instantiation). I understand that the generated path is cached, but that feels like an internal cache that doesnt need to be managed and guessed at. Why not just change the api like so:

const query = '<some query>'
const options = {}
// all the initialization steps are done here
const runQuery = JSONPath(query, options)

const object = {...}
const result = runQuery(object)

if the api worked like described, theres a lot of steps we can push onto to the initialization. Here are a few:

  • new JSONPath
  • all path parsing (though the JSONPath.toPathArray(expr) step is already cached)
  • vm.createScript(code) instead of vm.runInContext({ code })

I havent dug too deep into the code, so it may require redesign if the JSONPath class stores some state related to the json input. Is this an interesting idea to anyone?

@brettz9
Copy link
Collaborator

brettz9 commented May 18, 2019

Sounds like a great idea to me. If we're making a breaking change anyways, I think we can just make JSONPath into an ES6 class (in source), thereby requiring new, avoiding an extra step for this and getting rid of our NewError. runQuery would then be an instance method.

@brettz9
Copy link
Collaborator

brettz9 commented May 18, 2019

And if making breaking changes, we could also tie in #86.

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

No branches or pull requests

2 participants