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

Lazy module export #395

Closed CaptainN closed 4 years ago

CaptainN commented 5 years ago

Make module export lazy as per #394

aldeed commented 4 years ago

I guess this would be worth it, though I don't know why Meteor does it this way. It would be better if each project could just choose to depend on it lazily without requiring a breaking change to the package like this.

CaptainN commented 4 years ago

I agree. Meteor needs more of a peer dependency definition. This is probably a breaking change though - it would require some use cases do an intentional import. Maybe that's too much? IDK.

aldeed commented 4 years ago

I am transferring this to @Meteor-Community-Packages so I will let others help decide whether this is worth a major version release.

StorytellerCZ commented 4 years ago

I'm in agreement for major version update with this. Preferably with other changes updates like #250 so that there is a bit more goodies in the update.

harryadel commented 4 years ago

Thank you @StorytellerCZ. I was kinda skeptical whether to do a major or minor update, as for including #250 it seems that needs more work so I'm unsure about it.

StorytellerCZ commented 4 years ago

@harryadelb Yes, but that is out of scope for this PR. It is just matter of including things and timing the release. Anyhow @CaptainN could you please update your PR to fix the conflicts and do a major version bump. Thanks!

CaptainN commented 4 years ago

I actually think this would be too difficult for most users. We should probably not merge this.

copleykj commented 4 years ago

@CaptainN why would this be difficult?

CaptainN commented 4 years ago

Users could have to intentionally import the package before creating their collections. It would not auto attach to the collections the way it does now.

copleykj commented 4 years ago

What about either exporting Collection after it's modified so that it makes sense as to why you are importing something, or a combination of that and making use of the technique outlined by this article by @jankapunkt: https://dev.to/jankapunkt/lightweight-meteor-packages-with-conditional-dynamic-imports-1d51 so that it's optional?

CaptainN commented 4 years ago

I like the idea of exporting the modified collection (collection2) instead of modifying the original the way it does now. That's a better pattern in general, IMHO. It would allow for better code splitting and all that too.

But that's a HUGE change, and we'd need some wide discussion to catch any possible consequences that we aren't seeing. I generally support this idea though.

StorytellerCZ commented 4 years ago

@CaptainN I'm all for it. Maybe start a topic on the forums?