-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
Diffs when json key order changes #3583
Comments
Different order means its a different value. |
@B4nan could you reopen this one ? according to the json spec, it's not https://www.json.org/json-en.html
also, postgres |
So you say that the native implementation is not spec complaint, but that is rather a problem for TC39, right? JK :] I dont mind the proposed solution in #3584 (lets discuss the implementation there), but I dont think it should be the default if its slower than the native implementation. But depends on how much slower, if its like 20%, I dont mind, if its 200% percent, its a different story. |
yeah, fair point 😅
🙏
yeah, I tend to agree
No idea how much slower it is; I guess it is an acceptable compromise for those with the need to avoid unnecessary diffs. I also think there could be an other implementation if |
Not at all, and sorry from my end if my insta-close sometimes looks harsh, its just a way of me saying I dont plan to work on something as I feel its out of scope, especially if its something you can easily work around (like this particular issue, the proposed PR can be used without any changes to the ORM, you can just extend the
Thats a great idea! |
Describe the bug
If we update a json field with an identical value, only with a different key order, it's present in changeset (and thus triggers updates).
TLDR:
To Reproduce
Minimal full repro: https://stackblitz.com/edit/mikro-orm-repro-svx3gr?file=test.ts
Expected behavior
The JSON are identical, there should be nothing in changeset and no updates.
(or at least there should be an option to enable this behaviour)
I'm opening a PR with a suggested improvement right away.
Versions
The text was updated successfully, but these errors were encountered: