Upgrade packages in graphql pubspec.yaml and graphql_flutter pubspec.yaml

[hagen00]

Upgrade packages in graphql pubspec.yaml and graphql_flutter pubspec.yaml to latest, where possible.

[vincenzopalazzo]

Error: 84 tests passed, 2 failed.
--------------------------------------------------------------------------------

$ melos exec
  └> dart run test --coverage=coverage && dart run coverage:format_coverage --lcov --in=coverage --out=coverage.lcov --packages=.dart_tool/package_config.json --report-on=lib
     └> FAILED (in 1 packages)
        └> graphql (with exit code 1)

melos run client_test_coverage
  └> melos exec -- "dart run test --coverage="coverage" && dart run coverage:format_coverage --lcov --in=coverage --out=coverage.lcov --packages=.dart_tool/package_config.json --report-on=lib"
     └> FAILED
ScriptException: The script client_test_coverage failed to execute
make: *** [Makefile:38: ci_coverage_client] Error 1

probably there is some regression?

[hagen00]

If I remove the line expect(socketClient.socketChannel!.closeCode, isNotNull); in disconnect via dispose and disconnect via dispose graphql-transport-ws then all tests pass.

I'm unable to figure out why closeCode is now null. There seem to be a number of issues related to closeCode on web_socket_channel's Github issue tracker.

[vincenzopalazzo]

Unfortunatly I have not enough bandwidth to debug this issue, I do not want to promise that I will review, it because I am not in a good period, sorry!

Probably if I were you I would upgrade one dependency at a time(and make a single commit per dependency and per graphql package) in this way you can isolate what is going wrong.

[hagen00]

I have run some tests and it seems the problem lies with web_socket_channel no longer setting closeCode. I was able to reproduce this easily with the same code working on version 2.4.5 and not working on 3.0.1

I have submitted an issue: https://github.com/dart-lang/web_socket_channel/issues/383

[hagen00]

@vincenzopalazzo I have had a look at the source code and closeCode is not necessary for graphql-flutter to work. The websocket is still being closed successfully, it's just that the closeCode is not set, but by the looks of it, the graphql-flutter code does not require it.

Hence, I have removed expect(socketClient.socketChannel!.closeCode, isNotNull) in two places and now all tests pass.

The tests that check disconnect still have expects that indicate things working e.g this still passes fine:

await expectLater(
        socketClient.connectionState,
        emitsInOrder([
          SocketConnectionState.connected,
          SocketConnectionState.notConnected,
        ]),
      );

The benefit of having a new version of graphql-flutter, supporting web_socket_channel 3.0.1 are that code bases can now be upgraded, since a lot of packages now require web_socket_channel 3.0.1

Hopefully this can be merged now? The only thing that will still need updating (not sure how to do that) is to fix this:

dependencies:
  graphql: ^5.2.0-beta.8 # --> this will need to change on merge? Not sure. 

I will monitor https://github.com/dart-lang/web_socket_channel/issues/383 and if they fix closeCode (or indicate where the problem lies), I will update the tests again to check for this and do a new PR.

[hagen00]

I have changed the dependency in graphql_flutter from graphql: ^5.2.0-beta.8 to graphql: ^5.2.0-beta.9. I hope that was correct?

The version in graphql is still version: 5.2.0-beta.9. I'm not sure if that also needs incrementing i.e to version: 5.2.0-beta.10.

[vincenzopalazzo]

I have changed the dependency in graphql_flutter from graphql: ^5.2.0-beta.8 to graphql: ^5.2.0-beta.9. I hope that was correct?

This should not be required, it is the dependencies resolution that will fix it for you, In this way you are forcing everyone to upgrade the packages, that can be a huge pain something

The version in graphql is still version: 5.2.0-beta.9. I'm not sure if that also needs incrementing i.e to version: 5.2.0-beta.10.

package version are managed by the changelog tool, so leave them as it is.

This PR is missing just a big clean up inside the git history, 7 comments should be reduced in a smaller one.

A good reading https://cbea.ms/git-commit/

[hagen00]

@vincenzopalazzo I was trying to address this 2. Updagade the pubspec version in graphql_flutter. I have now reverted it to be as before, i.e graphql_flutter is now once again graphql: ^5.2.0-beta.8

I'm not entirely sure what else I need to do. If there are more changes required, please suggest them and I'll accept.

I have also rebased into a single commit.

[hagen00]

@vincenzopalazzo is there a problem with this PR? Is the formatting incorrect? Must I redo things so that there are no formatting changes? Is everything fine otherwise i.e 2 commits, with the 2 commit messages?