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

RequestControllerTypedLink return subscription streams that can end #629

Conversation

warrenisarobot
Copy link
Contributor

Update RequestControllerTypedLink so that it will not refetch and return the stream result directly. This allows the stream to end if the server completes the stream, and the listeners onData will fire.

Since the requestController stream never closes the switchMap's result stream will also never reach the onDone state. This uses only the first matching requestController emitted request when it is a subscription operation. This allows the switchMap's resulting stream to close when the server's response stream closes as well.

Copy link

netlify bot commented Feb 13, 2025

Deploy Preview for ferry-gql ready!

Name Link
🔨 Latest commit ac7b0cd
🔍 Latest deploy log https://app.netlify.com/sites/ferry-gql/deploys/67ae3a488c2c6800080ef9a6
😎 Deploy Preview https://deploy-preview-629--ferry-gql.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@warrenisarobot
Copy link
Contributor Author

Corresponding issue for this PR:

#630

Update RequestControllerTypedLink so that it will not refetch and
return the stream result directly.  This allows the stream to end if
the server completes the stream, and the listeners `onData` will fire.
@warrenisarobot warrenisarobot force-pushed the subscription-stream-close-on-server-close branch from 5b27925 to 32fe45e Compare February 13, 2025 17:35
@knaeckeKami
Copy link
Collaborator

Hi!

Please add a test that ensures that this new behavior will not break in the future.

should be something like

  group('done event for subscriptions', () {
    late TypedLink typedLink;

    setUp(() {
      typedLink = TypedLink.from([
        RequestControllerTypedLink(),
        TypedLink.function(<TData, TVars>(request, [next]) =>
            Stream<OperationResponse<TData, TVars>>.empty()),
      ]);
    });


    test('onDone is propagated for subscriptions', () {
      final stream = typedLink.request(
        JsonOperationRequest(
          fetchPolicy: FetchPolicy.NetworkOnly,
          vars: {},
          operation: Operation(
            document: gql.parseString(r'''
            subscription Sub {
                reviews {
                  id
                  stars
                }
            }'''),
          ),
        ),
      );

      expect(stream, emitsInOrder([emitsDone]));
    });
  });

Added test to verify that the subscription done event is emitted
through the RequestControllerTypedLink
@warrenisarobot
Copy link
Contributor Author

Hi!

Please add a test that ensures that this new behavior will not break in the future.

should be something like

  group('done event for subscriptions', () {
    late TypedLink typedLink;

    setUp(() {
      typedLink = TypedLink.from([
        RequestControllerTypedLink(),
        TypedLink.function(<TData, TVars>(request, [next]) =>
            Stream<OperationResponse<TData, TVars>>.empty()),
      ]);
    });


    test('onDone is propagated for subscriptions', () {
      final stream = typedLink.request(
        JsonOperationRequest(
          fetchPolicy: FetchPolicy.NetworkOnly,
          vars: {},
          operation: Operation(
            document: gql.parseString(r'''
            subscription Sub {
                reviews {
                  id
                  stars
                }
            }'''),
          ),
        ),
      );

      expect(stream, emitsInOrder([emitsDone]));
    });
  });

I appreciate the example on this one! I added the test to validate that the RequestControllerTypedLink is passing along the stream done

@knaeckeKami knaeckeKami merged commit 51961cb into gql-dart:master Feb 14, 2025
8 checks passed
@knaeckeKami
Copy link
Collaborator

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants