groq-js icon indicating copy to clipboard operation
groq-js copied to clipboard

feat: synchronous evaluator

Open ricokahler opened this issue 1 year ago • 2 comments

feat: Make evaluate synchronous and introduce toJS

Motivation

The SDK team utilizes groq-js to calculate user permissions based on GROQ filters applied to documents. This calculation needs to happen synchronously within the same frame where the document data is available, essentially making permissions a direct function of the current document state. The previous asynchronous nature of the evaluate function (returning a StreamValue with an async .get() method) made this pattern challenging to implement reliably.

Solution

This PR fundamentally rewrites the core evaluate function in groq-js to be synchronous.

  1. Synchronous evaluate: The evaluate function now directly returns an internal Value representation without involving Promises.
  2. Introduce toJS: Since evaluate returns an internal representation (which may include iterators or custom classes like DateTime), a new helper function toJS(value) has been introduced. This function eagerly converts the internal Value into a standard, plain JavaScript object, array, or primitive, resolving iterators and converting custom types (like DateTime to ISO 8601 strings).
  3. Simplified Implementation: The internal evaluator logic has been refactored and simplified, aiming for a more idiomatic JavaScript approach.
  4. Polyfills for Compatibility: To maintain compatibility with environments lacking certain newer JavaScript features (like Iterator.prototype.from), core-js-pure has been added as a dependency, and relevant polyfills are used internally. While this adds some code complexity, it's a trade-off for broader usability. This dependency could potentially be removed in the future as target environments evolve.

Key Changes

  • evaluate function is now synchronous.
  • Removed the asynchronous value.get() method previously used to resolve results.
  • Added a new exported function toJS(value) for converting evaluation results to plain JavaScript.
  • Updated README.md and API.md to reflect the new synchronous evaluate and toJS usage pattern.
  • Removed internal StreamValue implementation; now likely uses native iterators where applicable.
  • Internal DateTime class added/modified for date handling.
  • Added core-js-pure dependency and integrated necessary polyfills.
  • Significant refactoring of internal evaluation logic, operators, and functions to support the synchronous model.

Context & Collaboration

This change mostly aligns with discussions and a proof-of-concept (#273) reviewed by @judofyr. @judofyr agreed that a synchronous evaluator makes sense for a potential groq-js v2, allowing for an evaluateAsync to potentially be reintroduced later if needed.

I think this makes a lot of sense for a groq-js v2. We can still in a minor release introduce an evaluateAsync later on.

It seems to be pretty doable but handling dates requires checking a string to see if it's a date first and that may be problematic. At the current state of this PR, the date handling is close but still wrong.

I think we might need to keep our own Date wrapper, but there's a few things we can do:

  1. We can introduce an evaluation option called normalize (which defaults to true) which at the end traverses the object deeply and gets of rid any GROQ-specific types.
  2. We can implement toJSON() on our Date wrapper so that it's possible to run JSON.stringify(…) on it directly.

Let's also be way more precise (both in the API and preferably in the types) around what types we actually handle in the evaluator. We shouldn't need to use any / unknown anywhere.

  • The DateTime wrapper was kept ✅
  • We can introduce an evaluation option called normalize. I did not do this yet. Do you think it's still worth adding?
  • We can implement toJSON() on our Date This was done. ✅
  • Let's also be way more precise (both in the API and preferably in the types). This is mostly the case but I may have miss some functions that could have more strict return types. The Value type is meant to realize this vision.

API Changes Summary:

  • evaluate(tree, options): Now returns a synchronous internal Value.
  • value.get(): Removed.
  • toJS(value): New function to get plain JS data from the result of evaluate.

ricokahler avatar Apr 01 '25 00:04 ricokahler

[!WARNING] This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

ricokahler avatar Apr 01 '25 00:04 ricokahler

Very interested in this. Wanting to use groq-js in my corporate projects, but the async nature of the library makes implementation require too many updates to existing codebases. Thanks for doing this work @ricokahler 🎉

ldmichae avatar May 02 '25 13:05 ldmichae

I took a lot of inspiration of this, but added it in an opt-in manner in #299.

judofyr avatar Oct 20 '25 08:10 judofyr