Platform: EntityAdapter updateOne loses object type

Created on 17 Apr 2018  路  6Comments  路  Source: ngrx/platform

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Feature request
[ x] Documentation issue or request

I'm new to typescript, angular and ngrx, please pardon my ignorance.

What is the current behavior?

Adding item to store via Adapter maintains correct type where as on updateOne it loses its concrete type.

Expected behavior:

Would it be possible to maintain correct type? Or since we are passing changes as Partial<T> it is impossible?

Minimal reproduction of the problem with instructions:

// Class structure

abstract class Asset {
   assetId: string
}

// Concrete child classes
class BillAsset extends Asset {
}

class DocumentAsset extends Asset {
}

```
// Adapter Creation

export const adapter: EntityAdapter = createEntityAdapter({
selectId: (asset: Asset) => asset.assetId,
});


// Application Lifecycle

// When we add Assets it maintains correct object type in store like (BillAsset , DocumentAsset etc)
adapter.addMany(action.payload, state)

// Problem: Following causes item to change type from Asset to Object type
adapter.updateOne({ id: action.assetId,
changes: action.changes }, state)

```

Version of affected browser(s),operating system(s), npm, node and ngrx:

ngrx - v4.1.1

Other information:

Most helpful comment

In my opinion @ngrx/entity is more hindrance than help if it doesn't support "classes".
It's natural that I want to create some helper functions/getters on my entity objects. It's 2018 and JS provides such mechanisms, so why not to utilize them?

All 6 comments

This behavior is somewhat intentional. It is recommended that you only store plain objects in your Store, and it is required that entities managed @ngrx/entity are plain objects as well.

@MikeRyanDev Could you elaborate a little _why_ it is not recommended?

Not beeing able to store typed objects with nrgrx-entity forces us to not use oop-ish approaches in an environment that benefits significantly from classes and types.

EDIT:

Changing this line:
https://github.com/ngrx/platform/blob/8f05f1f0c24bdf84b65d18e243ccf62caf480d20/modules/entity/src/unsorted_state_adapter.ts#L84
to

const updated: T = Object.assign(original, update.changes);

would preserve the type

In my opinion @ngrx/entity is more hindrance than help if it doesn't support "classes".
It's natural that I want to create some helper functions/getters on my entity objects. It's 2018 and JS provides such mechanisms, so why not to utilize them?

I've just encountered this... not sure how this can be considered anything but a horrid bug, and I will have to rip entity out of my code.

Just created a PR (#1561) to call out this (arbitrary?) (update: reasoning is provided here https://github.com/ngrx/platform/issues/1376#issuecomment-431691078) limitation of entity state adapter in the docs. It doesn't appear to be mentioned anywhere and was very annoying and unexpected. Like @emulvihill, I'm going through and removing the dependency on ngrx/entity from my code. (update: the reasoning for this limitation is good, I'm now changing my code to adopt this best practice instead).

@thefliik
After working with ngrx and entity for several months I still consider this behavior as nothing else than arbitrary limitation. I already had several bugs because of this (I expected getter or function on the object but it wasn't there). As a solution I decided to project collections on the selectors level:

export const selectAllUsers = createSelector(
  getUsersState,
  selectAll
);
export const getUsers = createSelector(
  selectAllUsers ,
  users => users.map(user => new User(user));
);

I guess, nice side effect of this is that I double ensure that state is not modified outside the reducer, but I would rather not be forced to do this.

As for reasoning stated in #1376 I'm not convinced:

  1. I want logic in my objects, this is a whole point, let me take care of not mutating objects
  2. Not sure how preserving type would prevent it
  3. Not sure how preserving type would prevent it
Was this page helpful?
0 / 5 - 0 ratings