Store: Usage of arguments in Selectors

[piernik]

In 3.1 You introduced arguments in Selectors. How to use them? Here is my code which is not working.

@State<ConfigStateModel>({
  name: 'config',
  defaults: {
    clients: {
        schema: {}
    },
    incomes: {
        schema: {}
    },
  }
})
export class ConfigState {
  @Selector()
  static getSchema(type) {
    return (state) => {
      return state[type].schema;
    };
  }
}

Usage in component

export class ClientsListComponent {
    @Select(ConfigState.getSchema('clients'))
    schema$: Observable<any>;
}

[leon]

You spelled ConfigState.getSchemat wrong?

also state[type] could be null if type doesn't exist.

But we need more info on what is happening.
Could you please put together a stackblitz with your problem. It will make it much easier to debug and propose a solution.

[piernik]

I did misspelled but it was fast example and it is not an issue.

In standard @Select You pass reference to static method @Select(ConfigState.getClientsSchema) You do not execute it like @Select(ConfigState.getClientsSchema()).

How to pass argument to selector?
@Select(ConfigState.getSchema('clients')) is not working.

[juliusstoerrle]

I can't get it working either. It seems like the function returned by @Selector() is never called.

[evertonrobertoauler]

Same problem here, see my stackblitz to reproduce the issue.

https://stackblitz.com/edit/angular-nyzr88

[markwhitfeld]

There was a team misunderstanding over the signature of the dynamic selectors.
See this for original proposal:
https://github.com/ngxs/store/issues/386#issuecomment-390181710
It has now been corrected in the docs. See here:
https://ngxs.gitbook.io/ngxs/concepts/select#memoized-selectors
@amcdnl has also fixed the example in his announcement post.

Your example would be:

@State<ConfigStateModel>({
  name: 'config',
  defaults: {
    clients: {
        schema: {}
    },
    incomes: {
        schema: {}
    },
  }
})
export class ConfigState {
  @Selector()
  static getSchema(state) {
    return (type) => {
      return state[type].schema;
    };
  }
}

Usage in component

export class ClientsListComponent {
    @Select(ConfigState.getSchema)
    schemaFn$: Observable<(type: string) => any>;

    get clientsSchema$() {
        return this.schemaFn$.pipe(map(fn => fn('clients')));
    }        
}

[markwhitfeld]

@evertonrobertoauler I fixed your sample repro here:
https://stackblitz.com/edit/angular-selecto-param-repro-fix?file=src/app/app.component.ts

[markwhitfeld]

Just a thought for a convenience syntax for selection...
@amcdnl what do you think about this:

export class ClientsListComponent {
    @Select(ConfigState.getSchema, ['clients'])
    clientsSchema$: Observable<any>;
}

or provide a lambda for the map function (cannot access instance variables though, only static and constants):

export class ClientsListComponent {
    @Select(ConfigState.getSchema, (fn) => fn('clients') )
    clientsSchema$: Observable<any>;
}

[evertonrobertoauler]

@markwhitfeld Thanks for the sample fix and the docs update.

For a future major release and API, I believe this syntax would be better, because we get type checking and better tooling support:

export class ConfigState {
  @Selector()
  static getSchema(type: string) {
    return (state: ConfigStateModel) => {
      return state[type].schema;
    };
  }
}

export class ClientsListComponent {
  @Select(ConfigState.getSchema('clients'))
  clientsSchema$: Observable<any>;
}

But for now, this API syntax, I believe will be really handy, to reduce a bit of boilerplate

export class ClientsListComponent {
  @Select(ConfigState.getSchema, ['clients'])
  clientsSchema$: Observable<any>;
}

[piernik]

@markwhitfeld thanks for clarify.
I think Your new suggestion:

export class ClientsListComponent {
    @Select(ConfigState.getSchema, ['clients'])
    clientsSchema$: Observable<any>;
}

Is much, much better.

[markwhitfeld]

@evertonrobertoauler
Unfortunately the syntax you are requesting is incompatible with the convention for selectors and would break the paradigm. There is something that we could look at though to support this style as I definitely see the merits. Your code could potentially look like this:

export class ConfigState {
  @SelectorCreator()
  static getSchema(type: string) {
    return (state: ConfigStateModel) => {
      return state[type].schema;
    };
  }
}

export class ClientsListComponent {
  @Select(ConfigState.getSchema('clients'))
  clientsSchema$: Observable<any>;
}

This would allow for the 'inverted' syntax through the introduction of a new decorator SelectorCreator. It is essentially a factory method that is returning the selector hence the Creator suffix.
By comparison, the current syntax is a selector that returns a memoized function.
This is doable, but I would like to check with the team as to if the addition of another decorator is something we are happy to do.

[markwhitfeld]

cc @amcdnl @deebloo @leon
Hi team, could I get you input on these comments above:
https://github.com/ngxs/store/issues/413#issuecomment-394468056
https://github.com/ngxs/store/issues/413#issuecomment-394628288

[piernik]

@leon @amcdnl @deebloo configurable and easy @Select decorators with params (as @markwhitfeld sugested) are very needed. Please add them to ngxs

[evertonrobertoauler]

Thanks @markwhitfeld, I really liked this new decorator option @SelectorCreator() as a factory method to enable arguments, this would be really awesome and really easy to use, I truly believe that this approach would be nicer and simpler to use than @Select(ConfigState.getSchema, ['clients']) for this arguments functionality, with the benefit of simplifying the @Selector() implementation and API too.

[evertonrobertoauler]

@markwhitfeld @amcdnl @deebloo @leon

Hi team, here I developed an example of a possible implementation of this @SelectorCreator idea

import { Selector } from './selector';

export function SelectorCreator(selectors?: any[]) {
  return (target: any, key: string, descriptor: PropertyDescriptor) => {
    if (descriptor.value !== null) {
      return {
        configurable: true,
        get() {
          return function factory(...args) {
            const value = descriptor.value(...args);
            return Selector(selectors)(target, key, { ...descriptor, value }).get();
          };
        }
      };
    } else {
      throw new Error('SelectorCreators only work on methods');
    }
  };
}

Demo:

  // Selector with arguments
  @SelectorCreator()
  static valuePlus(value: number) {
    return (state: A2StateModel) => state.value + value;
  }

  // Joining Selectors
  @SelectorCreator([A1State.getValue])
  static valuePlusA1A2(value: number) {
    return (state: A2StateModel, a1Value: number) => {
      return state.value + value + a1Value;
    };
  }

  // Usage
  this.value2$ = this.store.select(A2State.valuePlus(10))
  this.value3$ = this.store.select(A2State.valuePlusA1A2(10))

And here I have a working and running demo of this new decorator using the latest @ngxs/store release

https://stackblitz.com/edit/angular-v89xad?embed=1&file=src/app/app.module.ts

I hope this could be useful and I'd love to contribute to this awesome project.

[markwhitfeld]

@evertonrobertoauler That looks great!
Could you create a PR for this?
If possible could you also add tests for this to the selector.spec.ts file with all the examples you can think of as part of your PR. Just follow the pattern set out by the existing tests.

I am still trying to get feedback from the rest of the team regarding this idea but it seems that they are all quite busy at the moment. I like it and I can see its' value.
cc @amcdnl @deebloo @leon

[evertonrobertoauler]

Thanks @markwhitfeld, I will create a PR with tests for this.

[amcdnl]

I'd like to try to make this work w/ the existing decorator rather than have to introduce another one. I'm going to play w/ it and see what I can come up w/.

[markwhitfeld]

@piernik This is now possible using the createSelector function just released in 3.2
Read PR #484 for details on usage.
I'll be updating the docs soon.

[piernik]

@markwhitfeld thanks.