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 for specific blocks #634

Closed
7 tasks
joluj opened this issue Jan 7, 2025 · 1 comment · Fixed by #635
Closed
7 tasks

[FEATURE] Add custom hook support for specific blocks #634

joluj opened this issue Jan 7, 2025 · 1 comment · Fixed by #635
Assignees
Labels
enhancement New feature or request

Comments

@joluj
Copy link
Contributor

joluj commented Jan 7, 2025

This is a follow-up issue of #633.

User Story

  1. As a {developer that uses the Jayvee lib}
  2. I want {to add custom hooks that run before or after blocks}
  3. So that {I can enrich Jayvee without modifying the original source code}

User Acceptance Criteria

  • I can add custom hooks to specific block types via reference to the class

  • I can add custom hooks to specific block types via a boolean function with the param { type: string } (or slightly different if it fits better to the current implementation)

  • The arguments (meta, input, output data) for the hook functions are typed properly.

    • This may be disabled for hooks with a blocks array of size >1. Here, the types may remain unknown.
  • The implementation should be backwards compatible, i.e. the blocks argument should be optional.

Examples

import { HttpExtractor } from '...';

myJayveeProgram.addPreBlockHook(myLambda, {
  blocks: [HttpExtractor],
})

// Or for custom blocks like composite blocks (they are not built-in)
myJayveeProgram.addPreBlockHook(myLambda, {
  blocks: [HttpExtractor, ({ type }) => type === 'MyCustomCompositeBlock'],
})

Notes

  • Try to keep it as simple as possible in regards to the implementation/design (which probably means it's not efficient as possible)
    • E.g., Map a missing list of blocks to [() => true]
    • E.g., Map a class that was given to the blocks array to a function internally (HttpExtractor => ({ type }) => type === HttpExtractor.type).

Definitions of Done

  • A PR has been opened and accepted
  • All user acceptance criteria are met
  • All tests are passing
@joluj joluj added the enhancement New feature or request label Jan 7, 2025
@TungstnBallon
Copy link
Contributor

There is currently no HttpExtractor exported from the interpreter library. An alternative would be to use HttpExtractorExecutor, but this assumes every block type only has one executor. While this is currently true (afaik), it might change in the future.

Resulting from a discussion with @joluj: We decided to use strings to represent block type names. This weakens the static typing for the hooks themselves (a hook on TableTransformer wouldn't have a static parameter of type Table), but it keeps the implementation simple.

New example:

myJayveeProgram.addHook('preBlock', myLambda, {
  blocktypes: 'HttpExtractor',
})

// Or for custom blocks like composite blocks (they are not built-in)
myJayveeProgram.addHook('preBlock', myLambda, {
  blocktypes: ['HttpExtractor', 'MyCustomCompositeBlock'],
})

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 a pull request may close this issue.

2 participants