[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?