Skip to content
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

Add undocumented/private API to hook into when a new frame is created. #701

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

adamscybot
Copy link
Contributor

@adamscybot adamscybot commented Jun 8, 2024

This is a small change to add a new hook, similar to __evaluate_entry and __evaluate_exit, but for when a new frame is created from an old one. I am building an open-source pluggable framework around JSonata, and this small change will open up an enormous amount of awesome possibilities.

It is beneficial for advanced cases like modifying env with additional metadata or mutating it so that you can analyse bind/lookup calls & provide diagnostics. I have actually patially achieved this already to significant effect by hooking into the env on __evaluate_entry. However, in some cases inside the JSONata core, a new frame is created, which is then immediately bound to, and consequently that happens before it has yet to recurse through evaluate and the entry callback. That means that callback is unable to capture everything for these advance pro-mode use cases. This new callback would enable that possibility.

I accept this will be undocumented and could change in the future.

The callback is set via a Symbol, so it can not be manipulated or read inside a query. See #700, which would need to be merged first, both as approval of the general Symbol approach and because the minor TS def changes are needed from there.

Signed-off-by: Adam Thomas adam@adam-thomas.co.uk

This is helpful in scenarios where you wish to debug the environment
behavior, or modify the env for advanced scenarios.

As the callback is set via a Symbol, it can not be manipulated
or read inside a query. See jsonata-js/jsonata/jsonata-js#700.
Copy link
Member

@andrew-coleman andrew-coleman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I think. @mattbaileyuk are you happy with this?

@mattbaileyuk
Copy link
Member

All looks good, including the protection around its use, so will merge this 👍

@mattbaileyuk mattbaileyuk merged commit 63e1a27 into jsonata-js:master Nov 28, 2024
@mattbaileyuk
Copy link
Member

Realised that didn't actually run through the checks; usually get an option on the merge to kick them off if they're initiated from a fork. Thankfully all looks good: https://github.com/jsonata-js/jsonata/actions/runs/12076711571/job/33678469127

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

Successfully merging this pull request may close these issues.

4 participants