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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃殌[FEATURE]: Allow Actions with non-static type fields #2092

Open
tobiso95 opened this issue Jan 11, 2024 · 2 comments
Open

馃殌[FEATURE]: Allow Actions with non-static type fields #2092

tobiso95 opened this issue Jan 11, 2024 · 2 comments

Comments

@tobiso95
Copy link

tobiso95 commented Jan 11, 2024

Relevant Package

This feature request is for @ngxs/store

Description

Actions should allow a non-static 'type' field

Instead of:

export class AddTodo {
  static readonly type = '[Todo] Add';
  constructor(public payload: any) {}
}

I would like to do the following:

export class AddTodo {
  readonly type = '[Todo] Add';
  constructor(public payload: any) {}
}

Describe the problem you are trying to solve

In your documenation it states that Actions should ideally follow a name pattern like this: [User API] GetUser.
Basically, [<Origin of the Event>] <Verb><Entity>
With the current implementation the only way to achieve this is by creating a separate class for every possible origin where this action could be dispatched from.

export class AddTodoFromOverview {
  static readonly type = '[TodoOverview] AddTodo';
  constructor(public payload: any) {}
}

export class AddTodoFromDetails {
  static readonly type = '[TodoDetails] AddTodo';
  constructor(public payload: any) {}
}
...

This gets incredibly verbose very quickly. I guess for this reason even the documentation does not continue to use this pattern but instead falls back to this:

export namespace Todo {
  export class Add {
    static readonly type = '[Todo] Add';
    constructor(public payload: any) {}
  }

  export class Edit {
    static readonly type = '[Todo] Edit';
    constructor(public payload: any) {}
  }
...
}

Describe the solution you'd like

I think something like this could be good:

class BaseAction {

  protected readonly _type: string;

  get type(): string {
    return `[${this._context}] ${this._type}`;
  }

  protected constructor(private _context: string) {
  }

}

export class AddTodoAction extends BaseAction {

  protected override _type = 'Add Todo'

  constructor(context: string, public todo: Todo) {
    super(context);
  }
}

// Dispatching from different components
this.store.dispatch(new AddTodoAction('Overview', {}));
this.store.dispatch(new AddTodoAction('Detail', {}));
this.store.dispatch(new AddTodoAction(TodoComponent.name, {}));

I'm sorry i have not dug deep into the sourcecode, but it seems that parts of ngxs already recognize non-static type fields.
I can dispatch the action, the @ngxs/devtools-plugin shows the name correctly, but creating an Actionhandler for this Action does not work:
TS2345: Argument of type typeof AddTodoAction is not assignable to parameter of type ActionType | ActionType[] Property type is missing in type typeof AddTodoAction but required in type ActionDef<any, any>

So, maybe this is just a bug

Describe alternatives you've considered

I've tried working around with static fields but i found no clean solution.

@markwhitfeld
Copy link
Member

markwhitfeld commented Jan 12, 2024

This is not a particularly common requirement and one that adds a layer of indirection that is not something that we would recommend for most codebases. We would recommend explicitly defined actions for traceability, debugability and clarity.

The other challenge is that the @Action(...) decorator is a compile-time constant, so you will need to solve how that can be cleanly provided to your state. I suspect that this side of the code will start to become a bit unwieldy with your approach.

I would recommend that you just include the context as part of the payload of the action and filter the action accordingly in state files.

That being said, it could be possible to achieve some of what you are looking for with a class factory function to create many different action classes from the same template.
I am not at my PC to test this, but it would be something like this (borrowing the classes you use above):

function createAddTodoActionClass(context: string) {
  return class AddTodoAction {
    static readonly type = '`[${context}] Add Todo`';

    constructor(public todo: Todo) {}
  }
}

export const AddOverviewTodoAction = createAddTodoActionClass('Overview');
export const AddDetailTodoAction = createAddTodoActionClass('Detail');

Applying similar principles, you could also achieve your dynamic case:

export class AddTodoAction {
  constructor(context: string, public todo: Todo) {
    const actionClass = createAddTodoActionClass(context);
    // returns a new object from the constructor function
    return new actionClass(todo);
  }
}

BUT, I would seriously consider making the dynamic context a part of your action payload instead.

@markwhitfeld
Copy link
Member

My creativity ran loose on this one...
I created a utility function that can take a normal action and add a prefix to the static type, as well as generate a new action type class when the function is called without new.
See this StackBlitz example with the utility function: https://stackblitz.com/edit/typescript-dynamic-action-type

You could use this createDynamicAction utility to convert a typical action type to any dynamic variant you like, and you could generate a static type to be used in your @Action(...) decorators.

It would be used as follows:

// in your action file
class AddTodoActionBase {
  static readonly type = `Add Todo`;
  constructor(public todo: Todo) {}
}

export const AddTodoAction = createDynamicAction(AddTodoActionBase);

// If you want to create explicit, known types
export const AddOverviewTodoAction = AddTodoAction('Overview');
export const AddDetailTodoAction = AddTodoAction('Detail');

Here are some examples of dispatching the various actions:

const t = new Todo();

store.dispatch(new AddOverviewTodoAction(t));
store.dispatch(new AddDetailTodoAction(t));
store.dispatch(new AddTodoAction('hi', t));

And in your state you can use the action decorator like this:

@Action(AddOverviewTodoAction)
addOverviewTodoAction( ctx /*... etc */) {}

@Action(AddDetailTodoAction)
addDetailTodoAction( ctx /*... etc */) {}

@Action(AddTodoAction('hi'))
addHiTodoAction( ctx /*... etc */) {}

Let me know if it helps you ;-)

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

No branches or pull requests

2 participants