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/filter params #30

Merged
merged 7 commits into from Jan 7, 2019

Conversation

Projects
None yet
2 participants
@JanTrotnow
Copy link
Collaborator

JanTrotnow commented Dec 5, 2018

I just added a few improvements for state/intent-filters. The idea is to allow parameterizing filters so that they are a little more flexible, e.g.

@filter({filter: ExampleFilter, params: {exampleParamKey: "exampleParamValue"}})
public async randomIntent(machine: Transitionable) {

For params you can pass an object containing any kind of key-values pairs which you are able to evaluate within the filter's execute-method.

The current way of using filters won't be affected by my changes. It's still possible to use them that way, e.g.

@filter(ExampleFilter)
public async randomIntent(machine: Transitionable) {

@JanTrotnow JanTrotnow requested a review from antoniusostermann Dec 5, 2018

@antoniusostermann
Copy link
Collaborator

antoniusostermann left a comment

Awesome contribution, thanks so much!

public execute: Hooks.BeforeIntentHook = async (mode, state, stateName, intent, machine, ...args) => {
this.logger.debug({ intent, state: stateName }, "Executing filter hook");

/** Prioritize filters by retrieving state filters first, followed by intent filters */
const prioritizedFilters = [...this.retrieveStateFiltersFromMetadata(state), ...this.retrieveIntentFiltersFromMetadata(state, intent)];

This comment has been minimized.

@antoniusostermann

antoniusostermann Dec 5, 2018

Collaborator

Type this correctly and avoid multiple blocks like

            hasParams
              ? (prioritizedFilter as { filter: Constructor<Filter>; params: { [key: string]: any } }).filter.name
              : (prioritizedFilter as Constructor<Filter>).name

Don't implement this check multiple times, check it once and store the filter in one variable and all parameters (or {}, if none) in another variable, independent of the original format.

@@ -3,7 +3,7 @@ import { Filter } from "./public-interfaces";

export const filterMetadataKey = Symbol("metadata-key: filter");

export function filter(...args: Array<Constructor<Filter>>) {
export function filter(...args: Array<Constructor<Filter> | { filter: Constructor<Filter>; params: { [key: string]: any } }>) {

This comment has been minimized.

@antoniusostermann

antoniusostermann Dec 5, 2018

Collaborator

It would be awesome if we could type the params in any way with a generic. Check if this possible, currently haven't an idea how.

@antoniusostermann antoniusostermann merged commit 475d9b6 into develop Jan 7, 2019

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@antoniusostermann antoniusostermann deleted the feature/filter-params branch Jan 7, 2019

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