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

Optional flag not working for sub-schemas #475

Closed
vpalos opened this issue Dec 24, 2022 · 13 comments
Closed

Optional flag not working for sub-schemas #475

vpalos opened this issue Dec 24, 2022 · 13 comments
Labels

Comments

@vpalos
Copy link

vpalos commented Dec 24, 2022

In version 3.4.0 of the package, setting optional: true on an object property which has its own required sub-fields, will enforce the object's sub-fields as required, even if the entire object is missing (which should be allowed, since the object itself is marked as optional).

See example below:

const addressSchema = new Schema({
    city: String,
});

const schema = new Schema({
    name: String,
    homeAddress: addressSchema,
    billingAddress: {
        type: addressSchema,
        optional: true,
    },
});

const context = schema.newContext();
context.validate({
  // EMPTY OBJECT
});

console.error(context.validationErrors());

The console output will show these errors:

0:{name: 'name', type: 'required', value: undefined} // OK
1:{name: 'homeAddress', type: 'required', value: undefined} // OK
2:{name: 'homeAddress.city', type: 'required', value: undefined} // OK
3:{name: 'billingAddress.city', type: 'required', value: undefined} // THIS IS WRONG!!!

Expected behavior:
The last error should not happen, of course because the entire billingAddress object is set as optional: true, therefore it's internal properties should only be enforced if (and only if) the object exists in the first place.

This expected behavior is also described in the documentation: https://github.com/longshotlabs/simpl-schema#optional

@github-actions
Copy link

Thank you for submitting an issue!

If this is a bug report, please be sure to include, at minimum, example code showing a small schema and any necessary calls with all their arguments, which will reproduce the issue. Even better, you can link to a saved online code editor example, where anyone can immediately run the code and see the issue.

If you are requesting a feature, include a code example of how you imagine it working if it were implemented.

If you need to edit your issue description, click the [...] and choose Edit.

Be patient. This is a free and freely licensed package that I maintain in my spare time. You may get a response in a day, but it could also take a month. If you benefit from this package and would like to see more of my time devoted to it, you can help by sponsoring.

@pozylon
Copy link

pozylon commented Dec 27, 2022

Same here, had to downgrade to 3.2.0, it's a regression

@loliarz
Copy link

loliarz commented Mar 17, 2023

I'm using nested objects validation a lot and also got this issue, are there any plans to fix that?

@jmdsg
Copy link

jmdsg commented Mar 19, 2023

Yeah, I am also facing the same issue. Are there any plans?

@herchu
Copy link

herchu commented Jun 22, 2023

I came across this issue as well. And after some debugging I found it's a change (a regression in my opinion) between 3.2.0 and 3.3.0.

Here is a sandbox to play with code, where you can switch between simpl-schema versions. I shortened the case to this schema:

const addressSchema = new SimpleSchema({
  city: String,
  street: String
});
const personSchema = new SimpleSchema({
  name: String,
  address: { type: addressSchema, optional: true }
});

Here, a person's address should be optional, but if it appears, it must contain a city and address. An object such as { name: 'Joe' } should pass validation, and it does in 3.2.0, but it does not in 3.3 and 3.4.

We found the issue while doing a round of package updates; we will stick for 3.2.0 for now, but I would appreciate if anyone knows why this was changed -- or otherwise, if there is any way to mark as optional nested objects with mandatory fields (I think that's a good description of what the issue is).

@red-meadow
Copy link

We are facing the same issue. Can the change be reversed?
@aldeed

@robbterr
Copy link

it´s somewhere in the commit e8f91dc for "feat: improved oneOf support" by @znewsham and @aldeed
but i can´t find the bug

@moonrockfamily
Copy link

moonrockfamily commented Nov 12, 2023

it´s somewhere in the commit e8f91dc for "feat: improved oneOf support" by @znewsham and @aldeed but i can´t find the bug

It seems the def.optional reference:

((def == null) || (def.optional !== true && childKeys.length > 0))
will never be evaluated as the def reference is only assigned within the for loop:
resulting in the temporary assignment of an {} empty object even when the definition has specified the object is optional!

@moonrockfamily
Copy link

Thought I'd share another gotcha with version 3.2.0 and its dependency on mongo-object 3.0.1 with a 'length' field bug! The below image shows the 'collection' variable value, with a length value and causing the 'while' loop to work away...

image

Kind of stuck! This latest version with the nested 'optional' misbehavior and the older version with its looping 'length' bug... lots of fun.

@moonrockfamily
Copy link

I'll open an issue on the mongo-object project for the each functions unexpected looping behavior when processing an object with length property but not really an array. This package's clean function is using mongo-object's removeArrayItems function which uses the mentioned each utility function.

Something like this isArrayLike function may solve it, but I'll need to write a bunch of tests first to contemplate the scenarios thoroughly to ensure the addition of the object has numeric keys condition is desirable.

/**
 * Checks if the given object is array-like, meaning it is not null or undefined, has a finite length, and has numeric keys.
 * @param {Object} obj - The object to check.
 * @returns {boolean} - True if the object is array-like, false otherwise.
 */
function isArrayLike(obj) {
  return obj != null && typeof obj === 'object' && isFinite(obj.length) && obj.length >= 0 && obj.length === Math.floor(obj.length) && obj.length < Number.MAX_SAFE_INTEGER && (obj[0] !== undefined || obj[obj.length-1] !== undefined || Object.keys(obj).length == 0);
}

@drone1
Copy link

drone1 commented Jan 26, 2024

Looks like it's been over a year since this was reported -- any plans to fix this? To not fix this? Any update or suggestions for a temporary fix would be greatly appreciated. Thanks.

@aldeed aldeed closed this as completed in 983ca42 Feb 26, 2024
Copy link

🎉 This issue has been resolved in version 3.4.6 🎉

The release is available on:

If this makes you happy, please consider becoming a sponsor.

Your semantic-release bot 📦🚀

@aldeed
Copy link
Collaborator

aldeed commented Feb 26, 2024

Thanks for your help debugging @moonrockfamily and others. I believe this should be fixed in 3.4.6

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

No branches or pull requests

10 participants