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

[FEATURE] Add custom hook support #635

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

[FEATURE] Add custom hook support #635

wants to merge 14 commits into from

Conversation

TungstnBallon
Copy link
Contributor

@TungstnBallon TungstnBallon commented Jan 10, 2025

Closes #633.

This PR adds hook functionality to the Jayvee interpreter. Hooks are custom functions that are automatically executed before / after blocks. See the PreBlockHook and PostBlockHook types for such a functions signature.

Library users can add hooks with the addHook method:

const program = /* parse program ...*/;

const before: PreBlockHook = (blocktype, input, context) => { /* ... */ };
program.addHook('preBlock', before, { blocking: true });

const after: PostBlockHook = (blocktype, input, context, output) => { /* ... */ };
program.addHook('postBlock', after);  // Options can be omitted. Non-blocking is the default

const blockSpecificHook = (blocktype, input, context) => { /* ... */ };
myJayveeProgram.addHook('preBlock', blockSpecificHook, {
  blocktypes: ['BuiltinBlockType', 'MyCustomCompositeBlock'],
})

// interpret model ...

Edit: Due to the simplifications outlined in #634 (comment) this PR also closes #634.

@TungstnBallon TungstnBallon requested a review from joluj January 10, 2025 13:38
@TungstnBallon TungstnBallon self-assigned this Jan 10, 2025
@TungstnBallon TungstnBallon added the enhancement New feature or request label Jan 10, 2025
Copy link
Contributor

@joluj joluj left a comment

Choose a reason for hiding this comment

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

Nice work!

I think that the logic will get quite complicated in the future with the current setup. But we can fix it as soon as it is a problem.

I'm curious why the pre and post hooks are not simply added in AbstractBlockExecutor::execute?

libs/execution/src/lib/hooks/hook-context.ts Outdated Show resolved Hide resolved
libs/execution/src/lib/hooks/hook-context.ts Show resolved Hide resolved
libs/interpreter-lib/src/interpreter.spec.ts Show resolved Hide resolved
libs/execution/src/lib/hooks/hook.ts Outdated Show resolved Hide resolved
libs/execution/src/lib/hooks/hook-context.ts Outdated Show resolved Hide resolved
@TungstnBallon
Copy link
Contributor Author

TungstnBallon commented Jan 14, 2025

I'm curious why the pre and post hooks are not simply added in AbstractBlockExecutor::execute?

I didn't want hooks to affect the block duration measured in executeBlock:

logExecutionDuration(startTime, executionContext.logger);

@TungstnBallon TungstnBallon requested a review from joluj January 14, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add custom hook support for specific blocks [FEATURE] Add custom hook support
2 participants