ngxs / store

🚀 NGXS - State Management for Angular
http://ngxs.io
MIT License
3.54k stars 403 forks source link

Use classes for models #700

Closed CROSP closed 5 years ago

CROSP commented 5 years ago

Hi, thank you for awesome library. I have one question regarding use of classes with the store.

In your documentation models are defined as interfaces. As follows

export interface ZebraFood {
  name: string;
  hay: number;
  carrots: number;
}

However consider the following case

I am defining my model as a class in order to be able to invoke methods.

export default class Profile {
    firstName: string;
    lastName: string;
    avatarUrl: string;
    email: string;
    role: string;
    permissions: Array;
    age: number;

    public getFullName() {
        return this.firstName + ' ' + this.lastName;
    }
}

And my state module looks as defined further

export class ProfileStateModel {
    currentUser: Profile;
}

@State<ProfileStateModel>({
    name: 'profile',
    defaults: {
        currentUser: null
    }
})
export class ProfileState {
    @Selector()
    static getCurrentUserProfile(state: ProfileStateModel) {
        return state.currentUser;
    }

    @Action(SetCurrentProfileUser)
    set({getState, patchState}: StateContext<ProfileStateModel>, {payload}: SetCurrentProfileUser) {
        patchState({
            currentUser: Object.assign({}, payload)
        });
    }
}

But when I request a state model from the store a simple object is returned instead.

    @Select(ProfileState.getCurrentUserProfile) currentUserProfile$: Observable<Profile>;
     this.currentUserProfile$.subscribe(value => {
            console.log('CURRENT PROFILE IS ');
            console.log(value);
            this.currentProfile = value;
        });

So I am not able to call any methods. Without any doubts this decision makes sense.

But what is the best way to have non anemic domain models? Should I create a model from the state object while fetching it?

       this.currentUserProfile$
            .map(value => Profile.createFromState(value))
            .subscribe(value => {
                console.log('CURRENT PROFILE IS ');
                console.log(value);
                this.currentProfile = value;
            });

Thanks in advance

viters commented 5 years ago

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

Your example is not working because you are using Object.assign({}, payload).

image

splincode commented 5 years ago

@CROSP I will soon update the documentation and show how this can be achieved.

use transforms in the selector with https://github.com/typestack/class-transformer

markwhitfeld commented 5 years ago

@CROSP Currently the framework expects all state to be object literals and treats them as such. This is typical for all redux type frameworks. This is mainly because of the guarantees of immutability that should be honoured by the state and also to ensure that the state can be persisted or transferred in and out of the store as necessary (storage, logger, and dev tools plugins). I would recommend doing the transformations that you require in your selectors.

CROSP commented 5 years ago

Thanks for answers, it is clear why all state objects and payload objects should be simple literals. I was just wondering wondering what are possible ways to handle that case. Thanks a lot.

markwhitfeld commented 5 years ago

We can leave this open until something is added to the docs about how to do this.

CROSP commented 5 years ago

Thanks, if somebody is also interested in that matter, here is a possible workaround, as suggested above.

import {plainToClass} from 'class-transformer';

export const ProfileMapper = (input: object) => {
    return plainToClass(Profile, input);
};

And we can just pass it as the map function argument.

 this.authService.onTokenChange()
            .pipe(
                map((value => value.getPayload())),
                map(ProfileMapper)
            )
            .subscribe(value => {
                this.store.dispatch(new SetCurrentProfileUser(value));
                console.log('TOKEN CHANGED ');
                console.log(value);
            });

However, don't misunderstand my purpose, I am not trying to move domain logic from backend to frontend, this is just for convenience and generally entity classes should be considered as a view model.

eranshmil commented 5 years ago

@CROSP As Mark replied above, I also think that this should be handled in the selector and not stored in the state.

ethan-far commented 5 years ago

Hi @eranshmil, I was wondering what that means for the code you have in ngxs-model-recipe, where you use classes as your models (e.g. the food.model.ts) ?

ethan-far commented 5 years ago

One question regarding naming convetions in regards to this question. The naming convention states that the model interfaces should not be suffixed with "Model". I understand the reason and I think it's a very good call. However, in the situation discussed here, I'd like the actual objects that the components/services are handling to have the concise names. If I use the concise names for the interfaces used by the store, I'll have to add a suffix that will be spread all across my code. What would you recommend in such a situation ? I was thinking about adding a suffix to the interfaces after all (e.g. Model, Entity, etc.).

arturovt commented 5 years ago

@markwhitfeld should we close this?

ethan-far commented 5 years ago

Yes. Thanks

On Thu, May 23, 2019, 01:43 Artur Androsovych notifications@github.com wrote:

@markwhitfeld https://github.com/markwhitfeld should we close this?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ngxs/store/issues/700?email_source=notifications&email_token=ABSIPEXIZQVN7XIS5PBOLCLPWXEAFA5CNFSM4GIEXQE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWAR5VI#issuecomment-495001301, or mute the thread https://github.com/notifications/unsubscribe-auth/ABSIPEX55W5EOUGNTFYCFJ3PWXEAFANCNFSM4GIEXQEQ .