search

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

Use meteor/ejson #396

Closed CaptainN closed 4 years ago

CaptainN commented 5 years ago

This PR simply reduces some of the constraints, and switches to the EJSON bundled with Meteor, instead of pulling in an additional dependency from npm.

aldeed commented 4 years ago

Why remove version constraints? I think it's best to have them to be clear about which packages work with which other packages.

CaptainN commented 4 years ago

Without version constraints these can be updated independently. With version constraints they can't. I think it's not ideal either way, but I generally like to keep things up to date.

It is possible to work around it the way it is now by adding a version over ride in the packages file.

aldeed commented 4 years ago

@CaptainN I don't use Meteor as much anymore, but the way it always worked was that these are simply minimum version constraints. A project can update to any higher minor or patch version using meteor update.

If there are newer major versions that are compatible, then they can be specified, too, with something like minimongo@1.0.0 || 2.0.0, which means "any version that starts with "1." or "2.". I still think this is best because I doubt this is compatible with versions before the ones that were listed (from early Meteor days). In practice maybe nobody would ever even try those versions, but it seems a small price to avoid the potential issue.

harryadel commented 4 years ago

I tend to agree with @aldeed here. We can readd the constraints removal and keep the rest as is. This and #395 would get merged as soon as we're sure we have the ability to publish to atmosphere.

harryadel commented 4 years ago

looking back on this change, the NPM version while adding more size to the package it much more updated than the Meteor one. I dunno, we might have to wait and see how things turn out.

copleykj commented 4 years ago

Personally I think if Meteor has a core package its good to use that. The Meteor version is tested to work with Meteor and while the updates may be good, they aren't guaranteed to not break compatibility in the future.

CaptainN commented 4 years ago

I removed the version constraints in an effort to reduce maintenance overhead. It's probably better to have them as long as we have someone willing to update and do the releases.

For EJSON, I was more concerned about bundle bloat from having more than one copy of the package. It seemed like low hanging fruit. If there is any important difference in output from the two packages, then care should be taken to avoid breaking applications with that change.