[Request] Simplify model updates

[GregOnNet]

Thank you

Hi,

thank you for providing this library. It is really intuitive and easy to use especially for newcomers dealing with state management.

Describe the feature you'd like:

Do you plan to enhance the Model-API making it easier to add, update and delete model items? The maintainers of ngrx/entity introduced an adapter for this. I did a lot with ngrx and would be glad to help to implement this feature. :)

Other information:

Desired Methods would be

• addOne
• addMany
• updateOne
• updateMany
• removeOne
• removeMany

I would be willing to submit a PR to add this feature:

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

If you are willing to submit a PR but are a bit unsure, feel free to check out the Contributors Guide for useful tips and hints that help you get started.

Cheers 🍻 Gregor

[tomastrajan]

Hi @GregOnNet !

Please, feel free to explore this topic and please share your progress so we can discuss what would make most sense ;)

[GregOnNet]

Thank you for the first feedback. Ich will start to work on this topic around the 22nd of Octobre since my schedule is a bit packed.

I will keep you up to date with my progress on this topic.

[GregOnNet]

Hey there,

I thought about a way to implement the mutator functions. First I want to get to know your opinion before I start hacking.

Array vs Dictionary vs Custom

The current example uses an Array-Type for the model. I could start to implement helpers for Array<T> but this would restrict users who want to go with Dictionary<T>.

From my perspective, there are two possibilities.

1. Add two methods createForCollection, createForDictionary to the ModelFactory.

2. Add only one method createWithAdapter that differentiates between Collection and Dictionary.

   //  model.ts
   createWithAdapter(initialData: T): Model<T> {
     return Array.isArray(initialData)
       ? new CollectionModel<T>(initialData, true, false)
       : new DictionaryModel<T>(initialData, true, false);
   }
   

   I would add a method to not break the existing behavior of your library.

If you find one of this proposals valuable, I am glad to provide a PR.

Cheers Gregor

[tomastrajan]

Hi @GregOnNet ! I think it makes sense because model supports both cases ( object and array ) so yeah would be great if the adapter supported both too! Also what about that hard-coded true, false ? We should preserve all configuration possibilities that come with original model, would that be possible?

[GregOnNet]

Hey,

thanks for the quick reply. Of cause, we can provide a configuration.

At the moment different methods are provided that configure Model differently. Instead of adding event more methods I suggest to introduce a configuration called ModelConfiguration:

EDIT 2

export interface ModelOptions {
  immuatable: boolean;
  sharedSubscription: boolean;
  clone: (model: T) => T;
  getIdientifier: (model: T) => string|number
}

I would also provide a default configuration based on the parameters which are passed in the method create:

const defaultModelOptions: ModelOptions = {
  immutable: true;
  sharedSubscription: false
}

In the end the API for createWithAdapter would look like this:

// with default options
this.model = this.modelFactory.createWithAdapter(initialData);

// with own options
this.model = this.modelFactory.createWithAdapter(initialData, { 
  immutable: false,
  sharedSubscription: true 
});

What do you think about it. Of cause now the API of createWithAdapter is a little bit different from the existing ones.

[tomastrajan]

@GregOnNet ok but add also possiblity to pass custom clone function as in original model, else all good, thank you!

[GregOnNet]

I missed one point. To provide mutation operations it is needed to know the key of an object. ngrx/entity defaults to id but you can configure it.

That's why I need to add an option enabling the user to pass a custom selector providing the identifier of the model.

export interface ModelOptions {
  immuatable: boolean;
  sharedSubscription: boolean;
  clone: (model: T) => T;
  getIdientifier: (model: T) => string|number
}

[tomastrajan]

Hey @GregOnNet ! How is it going ? Any progress on this feature? :)

[GregOnNet]

Hey @tomastrajan,

I was a bit busy the last two weeks but I already started to implement the feature. Unfortunately, I cannot say when I will finish my work on that. November is also quiet packet, but I will try my best! 👍

[GregOnNet]

Hey @tomastrajan,

I would like to discuss one thing before I go further.

From my perspective, it would be better to enforce immutable data structures using the Adapter for Collections and Dictionaries.

I started to implement the adapter functions for collections and it is really hard to preserve the reference of the Array.

If we agree to use immutable data structures only, I am also able to remove the methods clone and immutable form the ModelOptions.

For now, I do not really see a benefit supporting mutable data structures. But maybe I miss something.

What are your thoughts on this?

Kinds Greg

[GregOnNet]

Hi,

I just want to mention that I am still committed to this issue. I am working on another Angular Library right now, but since I have finished my current feature I will come back to this and send a PR. 🤗

Cheers Gregor

[tomastrajan]

Hi @GregOnNet that would be great!

[technbuzz]

Aren't we killing the purpose of minimal state management solution by adding ngrx/entity solutions?