Migrate/3.0 - fix: proposal to keep before.find operations sync

[vparpoil]

There is still a failing test on PR for the Async migration when using an Async function inside before.find hook (this test)

This issue source lies here where Async case is not handled.

I propose to update the code to throw an Exception

Due to the nature of before.find operations which modify a cursor before applying an Async method (fetchAsync, countAsync, ...), this seems the easiest way to fix the package and test suite and I don't see the limitation of not being able to use Async operation on before.find (which is mainly used to modify the selectors to implement soft deletion) as a huge drawback.

[vparpoil]

closing, following exchanges on the main PR

[jankapunkt]

Sorry I'm late to the show. I agree to keep it sync because it sticks with the API design of Mongo.Collection which I think we should be following strictly.

// cc @StorytellerCZ if you agree then please merge and let's continue in the migration 3.0 branch

[harryadel]

Hold your horses! I believe if we can find a way to maintain backwards compatibility while also allowing for async behavior for > 3.0 Meteor application then this is the best route instead of doubling down on one way to do it. Yes, it might require a bit of work on our end but I believe it'd be worth it.

[jankapunkt]

@harryadel but even with Meteor 3 the find method is never asynchronous so why bother in the first place?

[harryadel]

@harryadel but even with Meteor 3 the find method is never asynchronous so why bother in the first place?

Good point 🤔 I need to talk this out with you if possible on discord.

EDIT: I'll speak here instead so others can chime in as this is OSS after all.

Yes, Meteor 3.0 allows only for sync find operations but in your hooks you might want to do an async computation then pass whatever values you get back to the sync find operation. So, we don't need to blindly follow 3.0 on this one.

Though I'll also say that I'm partially driven by a need in our application where we modify a reactive value in our hooks which in turns makes all of our queries reactive. In testing we have this central management where if we were to modify/change the date, all of the values in our application have to respond to it. It's a little trick but serves us well.

I'm torn on this because on one hand, I don't wanna be selfishly trying to modify collection-hooks to fit our specific needs but on the other hand this is a real use case present in our application and could be present in other applications as well!

if (Meteor.isAppTest || Meteor.settings.public.manualTesting.enabled) {
    /**
     * Using dburles:mongo-collection-instances to get all registered collection instances.
     * I think this is still early enough to make all find()'s reactive, I think most view-relevant code will be ran later.
     *
     * We're then using the matb33:collection-hooks - package to add reactivity to each query on the client... madness! :D
     */
    Meteor.startup(() => {
        _.each(Mongo.Collection.getAll(), (collectionInfo) => {
            if (collectionInfo.name === '__test__fakeTime') return

            // todo AsyncTodo
            // NOTE: This actually isn't an array callback, but let's have lint have it's way...
            // eslint-disable-next-line array-callback-return
            collectionInfo.instance.before.find(async () => {
                const time = await getReactiveMoment_timeMachine()
            })
            collectionInfo.instance.before.findOne(async () => {
                const time = await getReactiveMoment_timeMachine()
            })
        })
    })
}

So, if you were in my shoe what would you do? @jankapunkt @vparpoil

[vparpoil]

Can you elaborate more on your use case? it looks like test code to me but I don't get the usage...

I personally don't see how this before find async hook is possible : find() is sync, there is no way to add an async before.find operation without changing how find() is working.

[harryadel]

After not so careful consideration 😅 , I'mma merge in your change and try to develop a solution for my specific problem later on (whether improving collection-hooks or implementing a custom fix).

Thank you @vparpoil for your contributions, you're one of the very few faces who offered help during 3.0 migration :)