Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

useQuery() is called outside of an active effect scope even tho i'm triggering useLazyQuery inside SFC composition API under <script setup> ... #1568

Closed
LukasGrubis opened this issue Aug 19, 2024 · 7 comments

Comments

@LukasGrubis
Copy link

LukasGrubis commented Aug 19, 2024

Describe the bug
When using useLazyQuery from @vue/apollo-composable within the Composition API in a Single File Component (SFC) under the <script setup> syntax, I receive a warning stating useQuery() is called outside of an active effect scope and the query will not be automatically stopped. This warning occurs despite the fact that useLazyQuery is triggered within a properly reactive context inside the setup function.

To Reproduce
Steps to reproduce the behavior:

  1. Create a Vue 3 component using the Composition API and <script setup> syntax.
  2. Import and use useLazyQuery from @vue/apollo-composable.
  3. Trigger useLazyQuery inside the setup function or in a method called within the setup context.
  4. Run the component and observe the console for warnings.
  5. The warning about useQuery() being called outside of an active effect scope appears.

Expected behavior
useLazyQuery should be recognized as being within the reactive context provided by the <script setup> in Vue 3. The query should execute without warnings, and the lifecycle management (e.g., automatic stopping of the query) should be handled by Vue.

Versions
vue: 3.4.26
@vue/apollo-composable: 4.1.0
@apollo/client: 3.10.8

Additional context
This issue appears even when useLazyQuery is directly placed inside the setup function in a standard Composition API pattern. The warning suggests that the function is outside of Vue’s reactive scope, despite being properly placed within setup. This behavior is unexpected, as the Composition API in Vue 3 should automatically manage the lifecycle of reactive hooks like useLazyQuery.

Additionally, similar warnings also occur when using useSubscription within the same reactive context. This indicates a broader issue with how @vue/apollo-composable methods are being integrated within the Vue 3 reactivity system.

@nickmessing
Copy link
Member

Hello @LukasGrubis, thank you for investing the time and effort into the proper creation of the bug report. Unfortunately I tried to reproduce what you are describing and failed locally. Could you provide a minimal reproduction if possible to help us investigate the issue further?

Note: Before the 4.1.0 the warning was not displayed, there's a chance that prior to 4.1.0 the issue was there but it was silent, starting with 4.1.0 we added a console message to make developers aware of the potential memory leak happening.

@LukasGrubis
Copy link
Author

LukasGrubis commented Aug 20, 2024

We are encountering a warning when using the useLazyQuery method from @vue/apollo-composable within the onMounted lifecycle hook to fetch user information based on a device's external ID in our Nuxt 3 project. Specifically, we first fetch data related to a device and then, using the retrieved device information, we proceed to fetch corresponding user data. The warning appears during the user data fetch (useLazyQuery), indicating that the query is being called outside of an active effect scope, despite being used within the setup context.

This issue might be specific to the Nuxt 3 framework, so it could be worth trying to reproduce this behavior in a Nuxt 3 environment as well. The warning could be related to how Vue 3's Composition API and Nuxt 3's specific configurations are integrating with @vue/apollo-composable, potentially due to the method not being properly recognized as being within a reactive context.

"nuxt": "^3.12.4",

Abstracted Code

<script setup>
import { useLazyQuery } from '@vue/apollo-composable';

import { SOME_DEVICE_QUERY } from '~/schemes/device/query';
import { SOME_USER_QUERY } from '~/schemes/user/query';

const deviceData = ref(null);
const userData = ref(null);

onMounted(async () => {
    const fetchDeviceData = async () => {
        const { result, load, error } = useLazyQuery(SOME_DEVICE_QUERY, {
            id: 'some-device-id',
        }, {
            fetchPolicy: 'no-cache',
        });

        await load();

        if (error.value) {
            return [];
        }

        deviceData.value = result.value.device;

        await fetchUserData();
    };

    const fetchUserData = async () => {
        const { result, load, error } = useLazyQuery(SOME_USER_QUERY, {
            id: 'some-user-id',
        }, {
            fetchPolicy: 'no-cache',
        });

        await load();

        if (error.value) {
            return [];
        }

        userData.value = result.value.user;
    };

    await fetchDeviceData();
});
</script>

<template>
    <div>
        <div>
            Device data loaded: {{ !!deviceData }}
        </div>
        <div>
            User data loaded: {{ !!userData }}
        </div>
    </div>
</template>

@nickmessing
Copy link
Member

nickmessing commented Aug 20, 2024

This would lead to a memory leak, that's a misuse that we are trying to notify people about with the new warning. The composition API should always be used inside the setup synchronously to let vue "know" about all the dependencies.

To achieve your desired behaviour while keeping the use* calls inside setup you can do:

<script setup>
import { computed } from 'vue';
import { useLazyQuery } from '@vue/apollo-composable';

import { SOME_DEVICE_QUERY } from '~/schemes/device/query';
import { SOME_USER_QUERY } from '~/schemes/user/query';

const {
    result: deviceDataResult,
    load: fetchDeviceData,
    error: deviceQueryError
} = useLazyQuery(SOME_DEVICE_QUERY, {
    id: 'some-device-id',
}, {
    fetchPolicy: 'no-cache',
});
const deviceData = computed(() => deviceQueryError.value ? [] : deviceDataResult.value.device)

const {
    result: userDataResult,
    load: fetchUserData,
    error: userQueryError
} = useLazyQuery(SOME_USER_QUERY, {
    id: 'some-user-id',
}, {
    fetchPolicy: 'no-cache',
});
const userData = computed(() => userQueryError.value ? [] : userDataResult.value.user)

onMounted(async () => {
    await fetchDeviceData();
    await fetchUserData();
});
</script>

<template>
    <div>
        <div>
            Device data loaded: {{ !!deviceData }}
        </div>
        <div>
            User data loaded: {{ !!userData }}
        </div>
    </div>
</template>

As you can see, we can call load inside onMounted however the use* things should live inside the "effect scope" which is synchronous during setup (and onMounted, but can't be called async)

In your scenario - useLazyQuery(SOME_DEVICE_QUERY) was "attached" to this component because it was called sync inside onMounted however the call to useLazyQuery(SOME_USER_QUERY) is happening only after the request to first query is done, that way there's no way for vue to know which component this is attached to since the "effect scope" is over.

The warning you are getting is just letting you know about a potential memory leak, the behaviour is not new, this only means that you had a memory leak before 4.1.0 that you never was made aware of.

@LukasGrubis
Copy link
Author

LukasGrubis commented Aug 20, 2024

useControlledSubscription.js

import { ref } from 'vue';
import { useSubscription } from '@vue/apollo-composable';

export function useControlledSubscription(subscriptionDocument, options = {}) {
    const subscriptionActive = ref(false);
    const subVariables = ref(null);
    let subscriptionControls;

    const loadingLocal = ref(false);
    const errorLocal = ref(null);

    function setupSubscription(variables) {
        try {
            const { loading, error, onResult, start, stop } = useSubscription(subscriptionDocument, variables);

            subscriptionControls = { start, stop };

            loadingLocal.value = loading;
            errorLocal.value = error;

            onResult(data => {
                if (options.onMessageReceived) {
                    options.onMessageReceived(data);
                }
            });
        } catch (error) {
            console.error('Subscription error', error);
        }
    }

    function startSubscription(variables = {}) {
        if (subscriptionActive.value) {
            stopSubscription();
        }

        subVariables.value = variables;

        setupSubscription(variables);
        subscriptionControls.start();
        subscriptionActive.value = true;
    }

    function stopSubscription() {
        if (subscriptionControls) {
            subscriptionControls.stop();
            subscriptionActive.value = false;
        }
    }

    function restartSubscription() {
        if (subVariables.value) {
            startSubscription(subVariables.value);
        }
    }

    return {
        error: errorLocal,
        loading: loadingLocal,
        startSubscription,
        stopSubscription,
        restartSubscription,
        subscriptionActive,
    };
}

useMessageSubscription.js

import { useControlledSubscription } from '~/composables/useControlledSubscription';
import { MESSAGE_CREATED_SUBSCRIPTION } from '~/schemes/message/subscription';

export function useMessageSubscription() {
    const {
        startSubscription,
        stopSubscription,
        restartSubscription,
        subscriptionActive,
        loading,
        error,
    } = useControlledSubscription(MESSAGE_CREATED_SUBSCRIPTION, {
        onMessageReceived: ({ messageCreated }) => {
            console.log('New message received:', messageCreated);
            // ...
        },
    });

    return {
        startSubscription,
        stopSubscription,
        restartSubscription,
        subscriptionActive,
        loading,
        error,
    };
}

This is a very simplified example of useControlledSubscription and useMessageSubscription to illustrate the core functionality and structure.

What We Are Doing:

Abstracting Subscription Logic: We’ve created a reusable composable called useControlledSubscription to manage the lifecycle of GraphQL subscriptions. This composable handles starting, stopping, and restarting subscriptions, as well as managing their loading and error states.

Specific Subscription Composable: We then create another composable, such as useMessageSubscription, which encapsulates the specific subscription schema (MESSAGE_CREATED_SUBSCRIPTION) and the logic for handling incoming messages (onMessageReceived).

Using the Composable in Components: In our components, such as app.vue, we only need to import and use the specific subscription composable, making it easy to manage subscriptions in a clean and modular way.

The Situation:

We are experiencing warning messages related to potential memory leaks. In our application, we have multiple subscriptions, and in app.vue, we use methods that contain collections of certain startSubscription triggers. Based on specific conditions, we trigger these methods to initiate the start of various subscriptions.

We are seeking guidance on what might be causing these warnings and if there's something we're overlooking in our approach. Could you provide a small example of how we might solve this differently to avoid these warnings? Your expertise on ensuring our subscription management aligns with best practices in Vue would be greatly appreciated.

@nickmessing
Copy link
Member

All the calls to use and on prefixed functions should happen synchronously during setup cycle, so you should always call them inside the root of your composition, not in a function (like setupSubscription). You can disable the subscription by default and enable inside a function.

Refactoring your entire code would take some time but to show you the gist of it:

// useControlledSubscription.js
export function useControlledSubscription(subscriptionDocument, options) {
  const isSubscriptionActive = ref(false)
  const subscriptionVariables = ref(null)
  
  const { loading, error, onResult } = useSubscription(
    subscriptionDocument,
    variables,
    () => ({
      enabled: isSubscriptionActive.value
    })
  )
  
  function setVariables(variables = {}) {
    subscriptionVariables.value = variables
  }
  
  function startSubscription(variables = {}) {
    setVariables(variables)
    isSubscriptionActive.value = true
  }
  
  function stopSubscription() {
    isSubscriptionActive.value = false
  }
  
  onDone(data => {
    options.onMessageReceived?.(data)
  })
  
  return {
    setVariables,
    startSubscription,
    stopSubscription,
    loading,
    error
  }
}

However, I would rather call useSubscription directly in the component that uses it with a boolean reference (ref(false)) for enable/disable control.

As this is a question and not really a bug report, prefer using official vue chat help forum. Feel free to ping me there for a more contextual discussion on how to solve your particular problem. (@nickmessing)

@Akryum Akryum closed this as completed Aug 21, 2024
@swunderlich
Copy link

swunderlich commented Sep 17, 2024

Hi,

we receive this warning inside a nuxt app in a middleware and face a memory leak in the server process.

Current implementation:

              const { onResult } = useUserDataQuery()
              onResult(({ data, loading }) => {
                if (!loading) {
                  ...result handling
                }
              })

Would using stop be a possible fix? (It would still show the warning though)

              const { onResult, stop } = useUserDataQuery()
              onResult(({ data, loading }) => {
                if (!loading) {
                  ...result handling
                }
                stop()
              })

Thanks in advance

@stephane303
Copy link

Hi,

we receive this warning inside a nuxt app in a middleware and face a memory leak in the server process.

Current implementation:

              const { onResult } = useUserDataQuery()
              onResult(({ data, loading }) => {
                if (!loading) {
                  ...result handling
                }
              })

Would using stop be a possible fix? (It would still show the warning though)

              const { onResult, stop } = useUserDataQuery()
              onResult(({ data, loading }) => {
                if (!loading) {
                  ...result handling
                }
                stop()
              })

Thanks in advance

I think I will do something like this, as I am not going to refactor my whole codebase, is there a way to suppress the warning ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants