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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Add example for Apollo GraphQL + composition API #192

Merged
merged 11 commits into from Dec 17, 2020

Conversation

agualis
Copy link
Contributor

@agualis agualis commented Dec 12, 2020

Updates the existing Graphql example to make it compatible with Vue 3 + @vue/apollo-composable.

The example needs a temporary patch until a pending PR is merged in vue-apollo project. That's why we have to use specific "@vue/apollo-composable": "4.0.0-alpha.10" version for now.

Implementation detail:

I had to create a wrapper component ComponentWithInjectedApollo to globally provide apollo client in the test. I would prefer not to do it and use some VTU option to call global.provide but I didn't find that option 馃し Maybe you know how to do it.

Bonus: I could add an additional test to check what happens when the Graphql request returns an error (It would involve adding some kind of error handling to the VueApollo.vue component).

@agualis
Copy link
Contributor Author

agualis commented Dec 12, 2020

Oops. Some tests are failing with newMapConsumer.eachMapping is not a function. First time I see this kind of error. I will wait to see if you have an explanation before starting investigating.

@afontcu
Copy link
Member

afontcu commented Dec 12, 2020

Hi, thanks for this!! 馃帀 this is great, it was one of the missing pieces to make sure VTL can support Vue 3, and I'm not experienced enough when it comes to vue apollo to make it work.

I would prefer not to do it and use some VTU option to call global.provide but I didn't find that option

It does exist: https://vue-test-utils.vuejs.org/v2/api/#global-provide

@afontcu afontcu added the vue3 label Dec 12, 2020
@agualis
Copy link
Contributor Author

agualis commented Dec 12, 2020

It does exist: https://vue-test-utils.vuejs.org/v2/api/#global-provide

I actually tried that but it didn't work. Maybe you can help to understand why...

@agualis
Copy link
Contributor Author

agualis commented Dec 12, 2020

it was one of the missing pieces to make sure VTL can support Vue 3

Happy if I can help with something else 馃槉

@afontcu
Copy link
Member

afontcu commented Dec 12, 2020

It does exist: https://vue-test-utils.vuejs.org/v2/api/#global-provide

I actually tried that but it didn't work. Maybe you can help to understand why...

Sure! Can you share what failed? I'm also on the VTU team so I can run the issue internally if needed 馃憤

@agualis
Copy link
Contributor Author

agualis commented Dec 12, 2020

Sure! Can you share what failed? I'm also on the VTU team so I can run the issue internally if needed 馃憤

If, instead of using ComponentWithInjectedApollo, you just render the Component like this:

  render(Component, {
      props: {id: '1'},
      global: {
        provide: {
          DefaultApolloClient: apolloClient,
        }
      }
  })

the apollo client won't be injected and you'll get this error when running the test:

Apollo Client with id default not found

@heywhy
Copy link
Contributor

heywhy commented Dec 13, 2020

Sure! Can you share what failed? I'm also on the VTU team so I can run the issue internally if needed 馃憤

If, instead of using ComponentWithInjectedApollo, you just render the Component like this:

  render(Component, {
      props: {id: '1'},
      global: {
        provide: {
          DefaultApolloClient: apolloClient,
        }
      }
  })

the apollo client won't be injected and you'll get this error when running the test:

Apollo Client with id default not found

You should have something like this 馃憞 instead:

import { DefaultApolloClient } from '@vue/apollo-composable'

...

render(Component, {
  props: {id: '1'},
  global: {
    provide: {
      [DefaultApolloClient]: apolloClient,
    }
  }
})

NB: I can prepare another PR if needed.

@heywhy
Copy link
Contributor

heywhy commented Dec 14, 2020

@agualis @afontcu You should refer to this Vue 3 Project Template for proper testing infrastructure support using @testing-library/vue. And you can clone the repo to start your own project 馃槃 . Though, I will love your feedbacks.

@agualis
Copy link
Contributor Author

agualis commented Dec 14, 2020

You should have something like this 馃憞 instead:

Oh. I missed that I needed square brackets because DefaultApolloClient is a symbol. Than you very much atanda!

Vue 3 Project Template

I looks good. Thanks for sharing.

@afontcu
Copy link
Member

afontcu commented Dec 15, 2020

Looks like we have some CI issues 馃 I ran tests locally and got the following error:

imatge

@agualis
Copy link
Contributor Author

agualis commented Dec 15, 2020

Defined is not defined. To be or not to be 馃槄

Which node version are you using? yarn or npm?

woml

@agualis
Copy link
Contributor Author

agualis commented Dec 15, 2020

I recall that I had to use a fixed apollo-composable version because I was having the same error (and we're not alone):

vuejs/apollo#1085

I'll try to reproduce it in another machine. It's funny that we are also having a different error in GHA CI 馃槄 (bug fragmentation 馃ぃ ).

@heywhy
Copy link
Contributor

heywhy commented Dec 15, 2020

@agualis I will like you to invite me to the repository you are working on because I'm able to resolve the failing tests and I don't want to create a separate PR for this project or for your fork. see loom recording below:

https://www.loom.com/share/abf53aaedce448d89564afc22c1ebfcf

cc: @afontcu

@agualis
Copy link
Contributor Author

agualis commented Dec 16, 2020

I will like you to invite me to the repository you are working on

I'm not sure if I'm following you @heywhy I don't have another project, just the fork.

@heywhy
Copy link
Contributor

heywhy commented Dec 16, 2020

I will like you to invite me to the repository you are working on

I'm not sure if I'm following you @heywhy I don't have another project, just the fork.

I don't want to create another fork of this project instead I want to push the changes directly to your own fork because I made the fixes on your fork but I don't have access to your fork so I can't push the changes. I hope that's clear

@afontcu
Copy link
Member

afontcu commented Dec 16, 2020

Looks like this PR contains changes that are already there in next. Can you make sure you check out from latest next changes? 馃

@heywhy
Copy link
Contributor

heywhy commented Dec 16, 2020

@afontcu I don't understand why the CI fails, can you help delete the cache for this PR workflow

@codecov
Copy link

codecov bot commented Dec 17, 2020

Codecov Report

Merging #192 (aabe84c) into next (49208ef) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              next      #192   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           91        91           
  Branches        30        30           
=========================================
  Hits            91        91           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 49208ef...aabe84c. Read the comment docs.

Copy link
Member

@afontcu afontcu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad you got CI working! Looks mergeable, right?

@heywhy
Copy link
Contributor

heywhy commented Dec 17, 2020

@agualis can you explain what went wrong??

@agualis
Copy link
Contributor Author

agualis commented Dec 17, 2020

Finally ready to merge 馃帄 . It seems that apollo+vue ecosystem is very active right now and this affected our PR.

@heywhy I enabled CI in my fork and I discovered that version 3.2.2 of apollo client was the one affecting our tests 馃槙 I didn't investigate the root cause but updating to the last stable one (3.3.6) fixed the problem.

@afontcu will keep an eye on this topic because apollo related dependencies will be fixed soon and we will be able to get rid of the nasty postinstall patch.

Thanks both for your patience and help 馃檹

@afontcu
Copy link
Member

afontcu commented Dec 17, 2020

Looks great :) Thank you for this!

@afontcu afontcu merged commit 94f57f8 into testing-library:next Dec 17, 2020
@github-actions
Copy link

github-actions bot commented Jan 8, 2021

馃帀 This PR is included in version 6.3.2 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

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

Successfully merging this pull request may close these issues.

None yet

3 participants