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

Standardize handling of null and undefined, and add tests for that. #39

Open
vitorgamer58 opened this issue Aug 22, 2023 · 1 comment
Open
Labels
enhancement New feature or request

Comments

@vitorgamer58
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We must standardize the treatment of null and undefined for fields in the database.
Nowadays we don't have tests that validate undefined fields in the toEntity method, but as reported in another issue, a null field is a value, and undefined is the lack of value, and we have encountered many problems when a key is undefined, as reported: #36 and #38

Describe the solution you'd like
Add unit tests to validate the rules related to returning null or undefined, using a validation that whenever the field is null or undefined, the field itself should be returned (i.e., not doing any conversion from null to undefined or undefined to null), as reported: #37 (comment)

Describe alternatives you've considered
I considered using checker.isEmpty, since #36 is a breaking change, so it wouldn't be a problem to have multiple breaking changes in a single PR, but the problem is that empty array ([]) and empty string ("") would also be considered empty, which could generate other problems.

Additional context
I believe that this issue would be better solved if it went up in a PR together with issues #36 and #38

@vitorgamer58 vitorgamer58 added the enhancement New feature or request label Aug 22, 2023
@vitorgamer58
Copy link
Contributor Author

I believe the solution should be as follows:
Has null or undefined been saved to the database? then the parser must return the value itself, without doing any checking.
Was saved empty array? Returns the empty array.

vitorgamer58 added a commit to vitorgamer58/herbs2mongo that referenced this issue Oct 21, 2023
Using checker.isEmpty null, undefined, {} or "" values will be returned by themselves, without being
converted to null or undefined and without being passed to the parser.

BREAKING CHANGE: This can change the way a value that does not exist in the collection is
undefined in the application that uses the lib.
A null value is returned as null, but an
undefined is passed to the parser and can return a different value.
Instead of "undefined",
undefined will be returned, causing breaking changes in systems that expect "undefined" with a
string type.

herbsjs#36 and herbsjs#39
vitorgamer58 added a commit to vitorgamer58/herbs2mongo that referenced this issue Oct 21, 2023
…y of entities

When an entity is empty (null, undefined or {}) it must return this value, without converting to
null

BREAKING CHANGE: These values were being converted to null previously, this could affect systems
that use the lib and do some if checking if it is equal to null, and with this change, it could
return undefined.

herbsjs#39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant