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

Nullsafe gql, gql_exec, gql_link #193

Closed
wants to merge 42 commits into from

Conversation

micimize
Copy link
Contributor

@micimize micimize commented Feb 13, 2021

This is a first pass at a nullsafe gql, mostly done with dart migrate and applied quick fixes. Idk what the status of nnbd-gql is – this was basically a "follow the instructions and see if tests pass" approach.

@micimize micimize requested a review from klavs February 13, 2021 18:31
@micimize micimize changed the title Nullsafe gql Nullsafe gql, gql_exec, gql_link Feb 13, 2021
@micimize
Copy link
Contributor Author

micimize commented Feb 13, 2021

Null safety migrations run thus far:

Libraries

  • cats
  • gql
  • gql_exec
  • gql_link
  • gql_http_link
  • gql_dedupe_link
  • gql_error_link
  • gql_transform_link
  • gql_websocket_link
  • gql_dio_link

blocked by dart-lang/build#2920

  • gql_code_builder
  • gql_build

Examples

  • gql_example_cli
  • gql_example_cli_github
  • gql_example_build
  • gql_example_http_auth_link
  • gql_example_dio_link
  • gql_example_flutter

@klavs
Copy link
Contributor

klavs commented Feb 13, 2021

Great work!
I can see this deployed as the initial nnbd version.
But before the final nnbd version is deployed, gql AST must be rechecked manually with GraphQL spec at hand. I know there are some fields on AST classes which are non-nullable and some are nullable.

I kind of forgot about the dart migrate when I did my initial work on nnbd (in some other branch)

@micimize
Copy link
Contributor Author

micimize commented Feb 14, 2021

migrated gql_http_link – discovered two things:

I'm thinking I should actually merge this into a gql-dart:nullsafety branch here and publish -nullsafety.alpha.0 versions manually as they're ready

@micimize micimize mentioned this pull request Feb 15, 2021
19 tasks
@micimize micimize closed this Feb 15, 2021
@klavs
Copy link
Contributor

klavs commented Feb 15, 2021

I'm thinking I should actually merge this into a gql-dart:nullsafety branch here and publish -nullsafety.alpha.0 versions manually as they're ready

I thought about the same thing because of the auto-release.

Copy link
Contributor

@klavs klavs left a comment

Choose a reason for hiding this comment

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

Almost every single use of ! operator is a potential for a bug :(

.github/check_package.sh Show resolved Hide resolved
.github/check_package.sh Show resolved Hide resolved
gql/lib/src/ast/transformer.dart Show resolved Hide resolved
gql/lib/src/ast/visitor.dart Show resolved Hide resolved
gql/lib/src/language/lexer.dart Show resolved Hide resolved
gql/lib/src/language/lexer.dart Show resolved Hide resolved
gql/lib/src/language/lexer.dart Show resolved Hide resolved
gql/lib/src/language/lexer.dart Show resolved Hide resolved
Token _expectToken(TokenKind kind, [String errorMessage]) {
final next = _next();
Token _expectToken(TokenKind kind, [String? errorMessage]) {
final next = _next()!;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably a bug as well.

gql/lib/src/language/printer.dart Show resolved Hide resolved
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