Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

apollo-composable "enabled/disable" query, not working #1422

Closed
tcastelly opened this issue Oct 28, 2022 · 11 comments
Closed

apollo-composable "enabled/disable" query, not working #1422

tcastelly opened this issue Oct 28, 2022 · 11 comments

Comments

@tcastelly
Copy link

Describe the bug
I would want to launch a query using the "enabled" property in the useQuery

const periodSelected = ref<string>();

const { result } = useQuery(REPORT, () => ({}), () => ({
  fetchPolicy: 'no-cache',
  enabled: !!periodSelected.value,
}));

By default the result of !!periodSelected.value is false. But the query is launched.

If I try to use a static false

const { result } = useQuery(REPORT, () => ({}), () => ({
  fetchPolicy: 'no-cache',
  enabled: false,
}));

It works.

Versions
vue: 3.2.41
vue-apollo: 3.1.0
@apollo/client: 3.7.1

Additional context
I've tried with computed options:

const { result } = useQuery(REPORT, () => ({}), computed(() => ({
  fetchPolicy: 'no-cache',
  enabled: !!periodSelected.value,
})));

Not working.

@rebz
Copy link

rebz commented Jan 30, 2023

Was running into this issue in a branch where I am upgrading from the following...

@apollo/client: 3.2.1
graphql: 14.6.0
webpack: 4.43.0
vue: 2.6.12
@vue/apollo-composable: 4.0.0-alpha.14

to...

@apollo/client: 3.7.4
graphql: 16.6.0
vite: 4.0.4
vue: 3.2.45
@vue/apollo-composable: 4.0.0-beta.1

Noticed something odd along with the query firing when it was disabled; the variables were blank on the outgoing query. After reading through the code one more time I realized my query's variable definitions were not properly set. Fixing the variable definitions in the query, to match the variables passed into useQuery solved everything. The query would no longer prematurely fire when disabled, and the variables showed up correctly when it did.

Guess what I'm saying here is, the latest version of apollo, graphql, vue, and this package work fine.

I'm curious if additional logic should be added in to check for a mismatch with variables and definitions. I would not have come to this thread if the query didn't fire while it was disabled.

@tcastelly - Was your issue solved?

@tcastelly
Copy link
Author

Bellow my code, the query is still trigger with undefined value. Maybe because the same variable is also used in the enabled property?

  const selectedPeriod = ref<string>();

  const { result: seriesResult, loading: isSeriesPending } = useQuery<Series>(
    REPORT,
    () => ({
      period: selectedPeriod.value,
    }),
    () => ({
      fetchPolicy: 'no-cache',
      enabled: !!selectedPeriod.value,
    }),
  );

  const unwatch = watch(
    () => periodOptions.value.length,  // `periodOptions` come from an other graph query
    () => {
      if (periodOptions.value.length && !selectedPeriod.value) {
        selectedPeriod.value = periodOptions.value[0].id;
        unwatch();
      }
    },
  );

The most wired behavior, if I never update the selectedPeriod.value

  const unwatch = watch(
    () => periodOptions.value.length,
    () => {
      if (periodOptions.value.length && !selectedPeriod.value) {
        // selectedPeriod.value = periodOptions.value[0].id;
        unwatch();
      }
    },
  );

the query is never trigger ...

So I tried with a setTimeout like this:

  const unwatch = watch(
    () => periodOptions.value.length,
    () => {
      if (periodOptions.value.length && !selectedPeriod.value) {
        setTimeout(() => {
          selectedPeriod.value = periodOptions.value[0].id;
        }, 0);
        unwatch();
      }
    },
  );

And it works.

I can't explain how it can works like this, I'm lost.

@Akryum
Copy link
Member

Akryum commented Feb 3, 2023

Hello! I'm not able to reproduce the issue on my end, could anyone try again with the latest versions of the libraries (vue, apollo, vue-apollo, etc.). 🙏

@tcastelly
Copy link
Author

Hello! I'm not able to reproduce the issue on my end, could anyone try again with the latest versions of the libraries (vue, apollo, vue-apollo, etc.). pray

Hello @Akryum
Thank you for your help.

I created a repo to reproduce the issue:
https://github.com/tcastelly/vuejs-apollo-issue

There is a workaround branch.

@yusufkandemir
Copy link

yusufkandemir commented Feb 9, 2023

I was on vue v3.2.37 and @vue/apollo-composable v4.0.0-alpha.16. Then, I upgraded to vue v3.2.47 (the latest), then started seeing this behavior. Then, I upgraded to @vue/apollo-composable v4.0.0-beta.2 and the behavior didn't change. Finally, I pinpointed the problem to vue v3.2.40. The problem is with vue v3.2.38. I have confirmed with the reproduction above, v3.2.37 works fine, but v3.2.38 and above don't work.

Related #1243 (comment)

@AubSs
Copy link

AubSs commented Feb 22, 2023

Hello, I hit the same problem,
here is a dirty fix until it's solved:

  1. Create a file post-install.sh with:
# Detect the platform
# /!\ If you're in MacOs, install gsed
case $(uname | tr '[:upper:]' '[:lower:]') in
  darwin*)
    alias sed=gsed
    ;;
esac

# https://github.com/vuejs/apollo/issues/1422
FILE="node_modules/@vue/apollo-composable/dist/index.esm.js"
sed -i '542 c nextTick(() => start());' $FILE
sed -i '796 c nextTick(() => start());' $FILE
  1. In the package.json scripts add:
    "postinstall": "sh post-install.sh"

  2. Remove node modules

  3. Re-install packages npm i

@thomergil
Copy link

thomergil commented Mar 6, 2024

@tcastelly, I also had this problem and figured it out. Note that enabled takes a Ref<boolean>, so, in your example, it's not !!periodSelect.value but !!periodSelect.

Building on your example, this is wrong:

const periodSelected = ref<string>();

const { result } = useQuery(REPORT, () => ({}), () => ({
  fetchPolicy: 'no-cache',
  enabled: !!periodSelected.value, // THIS IS WRONG
}));

This is what it should be:

const periodSelected = ref<string>();

const { result } = useQuery(REPORT, () => ({}), () => ({
  fetchPolicy: 'no-cache',
  enabled: !!periodSelected, // THIS IS RIGHT
}));

@tcastelly
Copy link
Author

@tcastelly, I also had this problem and figured it out. Note that enabled takes a Ref<boolean>, so, in your example, it's not !!periodSelect.value but !!periodSelect.

Hi,

You are right about how to use the useQuery. You can even use functions if you prefer:

 const {  result } = useQuery<Series>(
    REPORT,
    () => ({
      period: selectedPeriod.value,
    }),
    () => ({
      fetchPolicy: 'no-cache',
      enabled: !!selectedPeriod.value,
    }),
  );

But in the initial issue, it was not possible to use this enabled attribue correctly. Since it has been fixed.

@thomergil
Copy link

@tcastelly, thank you. To clarify, either of these work:

() => ({enabled: !!selectPeriod.value})

or

{enabled: !!selectPeriod}

but this does NOT work:

{enabled: !!selectPeriod.value}

@tcastelly
Copy link
Author

tcastelly commented Mar 6, 2024

@tcastelly, be that as it may--and I'm happy enabled got fixed--but your example is misleading. You should not dereference the Ref with enabled; that's where I got into trouble, whether I used a function or not. In your example it should be enabled: !!selectPeriod.

Let me clarify

useQuery can detect changes in parameters & options. You should be able to use:

  • directly a computed value (like you said: enabled: !!selectPeriod)
    but I think it's not a good practice
    image

  • or you can use a function and dereference the Ref
    image


About {enabled: !!selectPeriod.value} it should not be used. I totally agree. But in the initial issue,
{enabled: !!selectPeriod.value} and {enabled: false} does not have the same behavior.

@thomergil
Copy link

thomergil commented Mar 6, 2024

We agree 🙂. Thank you.

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

Successfully merging a pull request may close this issue.

6 participants