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

Typescript definitions #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

nikolayg
Copy link

Hey guys,

I've created some more accurate Typescript definitions for deep-object-diff. I would be happy if you merge them, so I can use them directly from the npm module :)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 71b28b8 on nikolayg:master into 6296889 on mattphillips:master.

@pzi
Copy link

pzi commented Oct 24, 2019

Awesome @nikolayg, thanks for this :)

@mattphillips would be awesome to get this merged, please :)

@jtabone16
Copy link

+1

@diyorbek
Copy link

diyorbek commented Aug 3, 2020

@nikolayg thanks for it).
It would be great to see the changes in the next release.

It is interesting why PR is still not merged.🤔

@Bessonov
Copy link

I'm not sure that it works this way. The originalObj and updatedObj could be fully unrelated and therefore typing both to T doesn't work. At least it's one possible scenario for diffing. I think a proper typings are possible, but 1. very hard to achieve 2. brings not much typesafety to the table.

@naseemkullah
Copy link

friendly ping @mattphillips

@AverageHelper
Copy link

AverageHelper commented May 3, 2021

I'm not sure that it works this way. The originalObj and updatedObj could be fully unrelated

@Bessonov I agree with you. This looks safe enough to me though, since each property of T gets exposed as optional, meaning that consumers will have to check that those values exist before using them. I'm not sure how this works in the case of adding properties though.

I've not searched the space much, but I imagine it's fairly uncommon to compare objects of two different types. In my own code at least, if I'm expecting a field to have been added to an object, that field would at least be defined on the type of the original as optional (potentially undefined). If the field exists on the new object, then great! I still need to check that it's undefined though since the original object is of the same type, and defines that field as optional.

@mattphillips
Copy link
Owner

I don't think these type defs are actually accurate, they don't recurse on Arrays and they error on the example usage in the project readme as shown in screenshot.

Screenshot 2022-11-12 at 12 47 45

I've taken a quick stab at writing some more advanced type definitions here: #88. If anyone wants to test drive them on a project and provide any issues/feedback on that PR that would be greatly appreciated.

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

Successfully merging this pull request may close these issues.

9 participants