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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NgRx Data .add() function can have an optional id parameter if isOptimistic: false but typing does not support that #2870

Closed
yharaskrik opened this issue Jan 9, 2021 · 1 comment · Fixed by #2899

Comments

@yharaskrik
Copy link
Contributor

Minimal reproduction of the bug/regression with instructions:

Any NgRx data entity that does not have it's primary key as optional, ex:

This will not allow you to call dataService.add({name: 'box'}, {isOptimistic: false})

interface Product {
  id: string
  name: string
}

This does though:

interface Product {
  id?: string
  name: string
}

Expected behaviour:

Might be a symptom of something else (providing explicit DTOs on a per implementation basic for each entity) but I don't think every property should be filled in for the add method since some defaults (like the id, or other things) could be set on the server. I don't think what I am asking is currently possible with TS since we do not know at compile time what the id property is since it can change based on the entity metadata.

This could be solved by providing another generic to the data service so that we could type the add method with an overload such as:

DataService<T, K> // T is type, K is primary key
add(entity: Partial<T>, options: Omit<EntityActionOptions, 'isOptimistic'> & {isOptimistic: false}): Observable<T> // not sure if this typing is right, let me know if it needs more explanation
add(entity: Required<Pick<T, K>> & Partial<T>, options?: EntityActionOptions): Observable<T> // modified current

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

Angular v11
NgRx v10

Other information:

I would be willing to submit a PR to fix this issue

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

@yharaskrik
Copy link
Contributor Author

Thinking more about this. I don't know if it is possible to type .add like I propose when the key is a composite, ie. SelectId function returns something other than a property from the type.

Maybe it just need to be Partial if isOptimistic:false?

yharaskrik added a commit to yharaskrik/platform that referenced this issue Jan 29, 2021
- Make the entity: T parameter for the NgRx Data .add command partial when isOptimistic: false since in some cases the PK will be created on the server

Closes ngrx#2870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants