Meteor-Community-Packages / meteor-collection2

A Meteor package that extends Mongo.Collection to provide support for specifying a schema and then validating against that schema when inserting and updating.
https://packosphere.com/aldeed/collection2
MIT License
1.02k stars 108 forks source link

Remove clean options #436

Closed harryadel closed 2 years ago

harryadel commented 2 years ago

https://github.com/Meteor-Community-Packages/meteor-collection2/pull/434#issuecomment-915454437

copleykj commented 2 years ago

Doing a quick search for cleanoptions in VS Code turns up quite a few references. If we are going to remove this, it should be done cleanly.

harryadel commented 2 years ago

@copleykj I ran a search with cleanoptions keyword, it did pull up some stuff but completely not related to this PR.

  schema.clean(doc, {
    mutate: true, // Clean the doc/modifier in place
    isModifier: (type !== "insert"),
    // The extend with the schema-level defaults (from SimpleSchema constructor options)
    ...(schema._cleanOptions || {}),
    // Finally, options for this specific operation should take precedence
    ...cleanOptionsForThisOperation,
    extendAutoValueContext, // This was extended separately above
    getAutoValues, // Force this override
  });
harryadel commented 2 years ago

Honestly, I hope we can bundle this PR with those two: https://github.com/Meteor-Community-Packages/meteor-collection2/pull/430 https://github.com/Meteor-Community-Packages/meteor-collection2/issues/433 (still in discussion)

and do a major bump, releasing 4.0.

copleykj commented 2 years ago

Ok, I'm gonna try to pull everything down and have a thorough look tomorrow or Monday. It's been quite a while since I've had my head in this codebase.

jankapunkt commented 2 years ago

@harryadel can yougive me a short intro how to test this, so I can add a review?

StorytellerCZ commented 2 years ago

I've slated it for v4 as it is a potential breaking change.

harryadel commented 2 years ago

@StorytellerCZ It's ok to include it in v4 if you wish but it isn't breaking since the introduced feature wasn't working or adding any value so removing it won't cause any problems whatsoever.

harryadel commented 2 years ago

I'm going to merge it but no won't release anything yet until we decided on what else to include in v4.

StorytellerCZ commented 2 years ago

Sorry for getting to this so late. We should have gotten that into its own v4 branch for better visibility.

harryadel commented 2 years ago

We should also account for https://github.com/meteor/meteor/pull/12057 collection2 must work with async operations, no?

StorytellerCZ commented 2 years ago

Yes, that should be the major thing for v4