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.

Sign up for GitHub

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

Jump to bottom

cf modules e2e test #1814

Merged
merged 3 commits into from
Sep 30, 2022
Merged

cf modules e2e test #1814

merged 3 commits into from
Sep 30, 2022

Conversation

saihaj
Copy link
Collaborator

@saihaj saihaj commented Sep 28, 2022 •
edited
Loading

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Sep 28, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: f8f1dd3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@saihaj saihaj changed the base branch from v2 to v3 September 28, 2022 13:34
@saihaj saihaj changed the title saihaj/debug module test cf modules e2e test Sep 28, 2022
dotansimha
dotansimha reviewed Sep 28, 2022
View reviewed changes
examples/cloudflare-modules/src/index.ts
export default {
fetch(request: Request, env: { [key: string]: string }) {
const yoga = createYoga({
graphqlEndpoint: env.GRAPHQL_ROUTE || '/graphql',
Copy link
Owner

Choose a reason for hiding this comment

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

We do landingPage: false and then GRAPHQL_ROUTE is injected to tell the Yoga server to expect incoming connections on e2e.graphql-yoga.com/WORKER_ID
This way we can have a public endpoint easily, without the need to capture /WORKER_ID/*.
It's a workaround, but it works for now...

ardatan
ardatan reviewed Sep 28, 2022
View reviewed changes
examples/cloudflare-modules/src/index.ts
export default { fetch }
export default {
fetch(request: Request, env: { [key: string]: string }) {
const yoga = createYoga({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe remove the parameter and modify the bundled code inside the deployment script;
https://github.com/dotansimha/graphql-yoga/pull/1812/files
Or do you think it is too tricky?

Copy link
Owner

Choose a reason for hiding this comment

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

I feel like it's too tricky... see my other comments on why we need this setup

Copy link
Collaborator

Choose a reason for hiding this comment

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

But this is also our example for CF Modules and useLandingPage and having yoga in the fetch function etc are too much for a basic example. We can have another project for e2e tests but we will lose the ability of testing the actual example in this case.

Copy link
Owner

Choose a reason for hiding this comment

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

I see, so let's move on with this PR as-is. Then, we'll open an issue to improve the setup. We'll need to experiment with using the router config that includes *, then we can use /WORKER_ID/graphql and keep the landing page enabled.

dotansimha
dotansimha reviewed Sep 28, 2022
View reviewed changes
examples/cloudflare-modules/src/index.ts
export default {
fetch(request: Request, env: { [key: string]: string }) {
const yoga = createYoga({
graphqlEndpoint: env.GRAPHQL_ROUTE || '/graphql',
Copy link
Owner

Choose a reason for hiding this comment

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

Also, CF module Worker doesn't use the globals for env vars, only through the env provided as part of fetch, so we need a thin wrapper in this case

Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of adding those extra options to our example, we can make it work by modifying the bundle like here;
#1812
What do you think @dotansimha @saihaj ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess we can do that too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the tricky part of it is just that is mutates the code example

@saihaj saihaj self-assigned this Sep 28, 2022
@github-actions
Copy link
Contributor

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-yoga/apollo-link 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/urql-exchange 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/common 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/redis-event-target 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/typed-event-target 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/graphiql 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
graphql-yoga 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/node 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/plugin-apollo-inline-trace 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/plugin-apq 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/plugin-persisted-operations 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/plugin-response-cache 1.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/render-graphiql 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎
@graphql-yoga/subscription 3.0.0-alpha-20220929105207-edf92781 npm ↗︎ unpkg ↗︎

@github-actions
Copy link
Contributor

✅ Benchmark Results

     ✓ no_errors
     ✓ expected_result

     checks.........................: 100.00% ✓ 136296      ✗ 0    
     data_received..................: 20 MB   677 kB/s
     data_sent......................: 7.8 MB  261 kB/s
     http_req_blocked...............: avg=1.11µs   min=700ns   med=1µs     max=549.5µs p(90)=1.4µs   p(95)=1.7µs   
     http_req_connecting............: avg=1ns      min=0s      med=0s      max=124.5µs p(90)=0s      p(95)=0s      
   ✓ http_req_duration..............: avg=372.16µs min=263.7µs med=338.5µs max=21.77ms p(90)=391.7µs p(95)=468.66µs
       { expected_response:true }...: avg=372.16µs min=263.7µs med=338.5µs max=21.77ms p(90)=391.7µs p(95)=468.66µs
     http_req_failed................: 0.00%   ✓ 0           ✗ 68148
     http_req_receiving.............: avg=17.28µs  min=9.5µs   med=16.1µs  max=10.06ms p(90)=19.2µs  p(95)=22.8µs  
     http_req_sending...............: avg=5.8µs    min=3.3µs   med=4.7µs   max=2.3ms   p(90)=6.6µs   p(95)=9.79µs  
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s      max=0s      p(90)=0s      p(95)=0s      
     http_req_waiting...............: avg=349.07µs min=246.6µs med=317.9µs max=21.66ms p(90)=366.3µs p(95)=434.06µs
     http_reqs......................: 68148   2271.482896/s
     iteration_duration.............: avg=435.77µs min=318.6µs med=400.5µs max=22.23ms p(90)=463.3µs p(95)=549.96µs
     iterations.....................: 68148   2271.482896/s
     vus............................: 1       min=1         max=1  
     vus_max........................: 1       min=1         max=1

@github-actions
Copy link
Contributor

🚀 Website Preview

The latest changes to the website are available as preview in: https://87dc35f3.graphql-yoga.pages.dev

@ardatan ardatan merged commit 5f2ffc1 into v3 Sep 30, 2022
@ardatan ardatan deleted the saihaj/debug-module-test branch September 30, 2022 07:07
@saihaj saihaj mentioned this pull request Oct 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotansimha dotansimha dotansimha approved these changes

@ardatan ardatan Awaiting requested review from ardatan

@enisdenjo enisdenjo Awaiting requested review from enisdenjo

Assignees

@saihaj saihaj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@saihaj @dotansimha @ardatan