Skip to content
This repository was archived by the owner on Oct 2, 2024. It is now read-only.

Opt in the null safety #259

Closed
wants to merge 8 commits into from
Closed

Conversation

glynskyi
Copy link

@glynskyi glynskyi commented Feb 8, 2021

Opt in the null safety

@comigor
Copy link
Owner

comigor commented Feb 18, 2021

Hello @glynskyi, thanks for this PR!

I think we still need to wait for all dependencies to be migrated before merging this PR and changing sdk requirements (specially gql, to avoid the force non-nullable ! calls); but this is a great start!

cc @vasilich6107 you were more involved in nnbd, what do you think?

@comigor comigor added the enhancement New feature or request label Feb 18, 2021
@vasilich6107
Copy link
Collaborator

@glynskyi thanks for PR.

@comigor we definitely need to check what's inside)

@jlnrrg
Copy link

jlnrrg commented Feb 19, 2021

For everyone like me interested in the progress, here is a list of the dependencies listed in pub.dev and their according nullsafety issues:

@featzima
Copy link

featzima commented Feb 19, 2021

Artemis is a code generator and it means that it is not included in the final build (only the generated classes). So it doesn't need to wait until all dependencies are migrates to the null safety. Actually I successfully uses this PR in my null safety projects.

@simolus3
Copy link

So it doesn't need to wait until all dependencies are migrates to the null safety

This is not strictly true. Users of code generators can mostly migrate before their dev_dependencies, yes. You could probably use the stable version of artemis in your null-safety projects too. The artemis project itself imports build-packages though, so it needs to wait for them to be migrated.

@jlnrrg
Copy link

jlnrrg commented Feb 19, 2021

@simolus3 you could, but there might be inconsistencies:
Lets take the generated Class GetUserArguments which would accept an int. Normally I could pass null as a value and everything works as intended, but now my projects variable I pass into is of type int? thus there is a missmatch.

@GP4cK
Copy link

GP4cK commented Feb 20, 2021

@featzima how do you do that:

I successfully uses this PR in my null safety projects.

I tried this in my pubspec.yaml:

dev_dependencies:
  artemis:
    git: https://github.com/glynskyi/artemis.git

But I get this error:

Because every version of artemis from git depends on equatable ^1.2.5 and [your_crappy_project] depends on equatable 2.0.0-nullsafety.3, artemis from git is forbidden.

I tried running flutter clean and even deleting the pubspec.lock but it didn't fix it...

@featzima
Copy link

@GP4cK You're able to fix the required library version during the transition period. Just create the dependency_overrides section in your pubspec.yaml file. The list of required libraries depends on your dependencies. My pubspec.yaml looks like:

dependency_overrides:
  analyzer: ^0.41.2
  collection: ^1.15.0-nullsafety.4
  rxdart: ^0.24.0
  graphql: ^4.0.0-beta
  gql: '>=0.12.3 <1.0.0'

@jlnrrg
Copy link

jlnrrg commented Feb 20, 2021

@featzima interesting 🤔. How do you not get the glob issue?
Can you by change post your whole pubspec.yaml


Because gql_code_gen 0.1.5 depends on glob ^1.2.0 and no versions of gql_code_gen match >0.1.5 <0.2.0, gql_code_gen ^0.1.5 requires glob ^1.2.0.

And because every version of artemis from git depends on gql_code_gen ^0.1.5, every version of artemis from git requires glob ^1.2.0.

So, because TestProject depends on artemis from git which depends on glob ^2.0.0-nullsafety.0, version solving failed.
pub get failed (1; So, because TestProject depends on artemis from git which depends on glob ^2.0.0-nullsafety.0, version solving failed.)
exit code 1

(also forcing glob to version ^2.0.0-nullsafety.0 leads to issues on code generation)

@featzima
Copy link

featzima commented Feb 20, 2021

@jlnrrg

pubspec.yaml
environment:
  sdk: '>=2.12.0-0.0 <3.0.0'

dependencies:
  flutter:
    sdk: flutter
  cupertino_icons: ^1.0.2
  vector_math: ^2.1.0-nullsafety.5
  path: ^1.8.0-nullsafety.3
  intl: ^0.17.0
  rxdart: ^0.24.0
  sprintf: ^6.0.0-nullsafety
  shared_preferences: ^0.5.12+4
  dio: ^3.0.10
  built_value: ^8.0.0-nullsafety.0
  built_collection: ^5.0.0-nullsafety.0
  http_multi_server: ^2.2.0
  flutter_bloc: ^7.0.0-nullsafety.1
  expandable: ^4.1.4
  scroll_to_index: ^1.0.6
  geolocator: ^7.0.0-nullsafety.5
  geocoding: ^1.0.5
  synchronized: ^3.0.0-nullsafety.1
  vicodin: ^1.0.5-nullsafety.0
  ktx: ^1.1.5-nullsafety
  shimmer: ^2.0.0-nullsafety.0
  sealed_unions: ^3.0.2+2
  flutter_email_sender: ^4.0.0
  url_launcher: ^6.0.0-nullsafety.6
  package_info: ^0.5.0-nullsafety
  flutter_native_timezone: ^1.0.4
  freezed_annotation: ^0.13.0-nullsafety.0
  timezone: ^0.7.0-nullsafety.0
  flinq: ^1.1.1
  trotter: ^1.2.0
  firebase_core: ^0.8.0-1.0.nullsafety.1
  firebase_analytics: ^7.0.1
  firebase_crashlytics: ^0.5.0-1.0.nullsafety.2
  flutter_vibrate: ^1.0.0
  floor: ^1.0.0-nullsafety.0
  rate_app_dialog: ^0.1.2
  google_maps_flutter: ^1.0.6
  equatable: ^2.0.0-nullsafety.3
  app_settings: ^4.0.4
  countdown_flutter: ^0.1.2
  flutter_svg: ^0.20.0-nullsafety.3
  auto_size_text: ^3.0.0-nullsafety.0
  fimber: ^0.5.0-nullsafety.1
  collection: ^1.15.0-nullsafety.4

dependency_overrides:
  analyzer: ^0.41.2
  collection: ^1.15.0-nullsafety.4
  clock: ^1.1.0-nullsafety.3
  intl: ^0.17.0
  path: ^1.8.0
  url_launcher: ^6.0.0-nullsafety.6
  glob: ^2.0.0
  firebase_core: ^0.8.0-1.0.nullsafety.1
  plugin_platform_interface: ^1.1.0-nullsafety.2
  meta: ^1.3.0-nullsafety.6
  ffi: ^0.1.3
  sqlite3: ^0.1.8
  rxdart: ^0.24.0
  graphql: ^4.0.0-beta
  gql: '>=0.12.3 <1.0.0'

dev_dependencies:
  flutter_test:
    sdk: flutter
  build_runner: ^1.11.1
  built_value_generator: ^8.0.0-nullsafety.0
  floor_generator: ^1.0.0-nullsafety.0
  freezed: ^0.13.0-nullsafety.0
  artemis:
    git:
      url: https://github.com/glynskyi/artemis.git
      ref: 68a83f3
  json_serializable: ^4.0.0

@vasilich6107
Copy link
Collaborator

vasilich6107 commented Mar 3, 2021

@jlnrrg meta supports nullsafety from 1.3.0

Scroll this page to the bottom
https://pub.dev/packages/meta/versions

So we can switch on the checkbox

collection is from v1.15.0
https://pub.dev/packages/collection/versions

@vasilich6107 vasilich6107 mentioned this pull request Mar 10, 2021
@LasseRosenow
Copy link
Contributor

LasseRosenow commented Mar 14, 2021

build 2.0.0 with nullsafety was released ;)

@jlnrrg

@comigor comigor mentioned this pull request Mar 16, 2021
pubspec.yaml Outdated
gql: ^0.13.0-nullsafety.1
code_builder: ^3.6.0
collection: ^1.15.0
dart_style: ^1.3.14
Copy link

Choose a reason for hiding this comment

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

Suggested change
dart_style: ^1.3.14
dart_style: ^2.0.0

@comigor
Copy link
Owner

comigor commented Mar 18, 2021

Hey all, we've just released 7.0.0-beta.1 as a breaking change, still in beta, version that outputs Artemis generated code as nnbd (eg. GraphQL Int! becomes Dart (late) int, while GraphQL Int becomes Dart int?).

Soon I'll start working on migrating Artemis code itself to nnbd, probably rebasing this PR if it's not too much work (given the amount of conflicts – sorry about that, but I lot had to be changed to output code to nnbd), following the dependencies and maybe we can have a full nnbd prerelease.

But please, if your project is already using nnbd, test this new beta and report bugs on top of it! It'd be very helpful to get real use cases (my company has not migrated yet!).

@jlnrrg
Copy link

jlnrrg commented Mar 25, 2021

@comigor
I think it would be a good idea to adapt these version too

  • yaml: ^3.0.0 (analyzer >=1.0.0 depends on it)
  • source_gen: ^1.0.0
  • glob: ^2.0.0
  • dart_style: ^2.0.0
  • build: ^2.0.0
  • http: ^0.13.0
  • gql_link: ^0.4.0-nullsafety.2
  • gql_http_link: ^0.4.0-nullsafety.1
  • gql_exec: ^0.3.0-nullsafety.1
    At this point I stopped comparing step by step and just reference to the nullsafety version of graphql.
    And I also link my comment again, where the packages with nullsafety are listed

@inc16sec
Copy link

inc16sec commented Mar 30, 2021

Is there any progress with this new update (null-safety), still waiting for the packages required by Artemis to be updated.
@jlnrrg have already pointed out most of the packages that need to be updated to the latest release otherwise a build conflict error stops the whole build.

@comigor
Copy link
Owner

comigor commented Mar 30, 2021

@inc16sec Not yet, I didn't have time to adapt the package itself to nnbd. However, since 7.0.0-beta.1 it's possible to generate nnbd code, and technically one could update dependency_overrides as featzima's comment above and move on (I'd like people to test it and report back, if possible).

@jlnrrg
Copy link

jlnrrg commented Mar 30, 2021

@comigor thanks for clarifying.
I personally would love to test for errors, but as I highly depend on artemis, I do not want to override so much dependencys with major version changes, as it creates possible conflicts down the line.
also it is a tedious entry barrier for people wanting to test the new nnbd version.

If possible I'd like you to reconsider updating the packages first, thus gaining more testing people 😅

if that is not possible without completely switching to nnbd, then nvm my bad.

@comigor
Copy link
Owner

comigor commented Mar 30, 2021

@jlnrrg Yeah, I'm not waiting for people to test before upgrade, I just didn't had the time to do it yet 😅

@inc16sec
Copy link

@comigor so I have just tried to override these dependencies:

dependency_overrides:
  build: ^2.0.0
  dart_style: '>=1.3.13 <=1.3.13'
  glob: ^2.0.0
  gql: ^0.13.0-nullsafety.2
  gql_exec: ^0.3.0-nullsafety.1
  gql_http_link: ^0.4.0-nullsafety.1
  gql_dedupe_link: ^2.0.0-nullsafety.1
  gql_link: ^0.4.0-nullsafety.2
  graphql: 5.0.0-nullsafety.1
  http: ^0.13.0
  source_gen: ^1.0.0
  yaml: ^3.0.0

And I called flutter pub run build_runner watch --delete-conflicting-outputs but this error showed up.

[INFO] Generating build script...
[INFO] Generating build script completed, took 625ms

[INFO] Creating build script snapshot......
[WARNING] stderr: ../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql_code_gen-0.1.5/lib/ast_builder.dart:83:11: Error: The getter 'name' isn't defined for the class 'DefinitionNode'.
[WARNING] stderr:  - 'DefinitionNode' is from 'package:gql/src/ast/ast.dart' ('../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql-0.13.0-nullsafety.2/lib/src/ast/ast.dart').
[WARNING] stderr: Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
[WARNING] stderr:   if (def.name != null && def.name.value != null) return def.name.value;
[WARNING] stderr:           ^^^^
[WARNING] stderr: ../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql_code_gen-0.1.5/lib/ast_builder.dart:83:31: Error: The getter 'name' isn't defined for the class 'DefinitionNode'.
[WARNING] stderr:  - 'DefinitionNode' is from 'package:gql/src/ast/ast.dart' ('../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql-0.13.0-nullsafety.2/lib/src/ast/ast.dart').
[WARNING] stderr: Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
[WARNING] stderr:   if (def.name != null && def.name.value != null) return def.name.value;
[WARNING] stderr:                               ^^^^
[WARNING] stderr: ../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql_code_gen-0.1.5/lib/ast_builder.dart:83:62: Error: The getter 'name' isn't defined for the class 'DefinitionNode'.
[WARNING] stderr:  - 'DefinitionNode' is from 'package:gql/src/ast/ast.dart' ('../../../../documents/flutter/.pub-cache/hosted/pub.dartlang.org/gql-0.13.0-nullsafety.2/lib/src/ast/ast.dart').
[WARNING] stderr: Try correcting the name to the name of an existing getter, or defining a getter or field named 'name'.
[WARNING] stderr:   if (def.name != null && def.name.value != null) return def.name.value;
[WARNING] stderr:                                                              ^^^^
[INFO] Creating build script snapshot... completed, took 10.4s

[SEVERE] Failed to snapshot build script .dart_tool/build/entrypoint/build.dart.
This is likely caused by a misconfigured builder definition.

pub finished with exit code 78

And these are the packages that I'm using:

dependencies:
  flutter:
    sdk: flutter

  firebase_auth: ^1.0.1
  # freezed_annotation: ^0.14.1
  flutter_bloc: ^7.0.0
  firebase_core: ^1.0.2
  firebase_storage: ^8.0.1

  graphql_flutter: ^5.0.0-nullsafety.1

  flutter_hooks: ^0.16.0
  hooks_riverpod: ^0.13.1+1

  universal_platform: ^1.0.0-nullsafety

  json_serializable: ^4.1.0
  built_value: ^8.0.4
  built_collection: ^5.0.0
  gql: ^0.13.0-nullsafety.2


  # The following adds the Cupertino Icons font to your application.
  # Use with the CupertinoIcons class for iOS style icons.
  cupertino_icons: ^1.0.2

@comigor
Copy link
Owner

comigor commented Mar 31, 2021

@inc16sec Hmmm so some breaking changes may be really breaking now :(

@tlvenn
Copy link

tlvenn commented Mar 31, 2021

@inc16sec I had the same issue, i got it running for now keeping gql with the latest 0.12.x

@inc16sec
Copy link

@inc16sec I had the same issue, i got it running for now keeping gql with the latest 0.12.x

I have just tried it with gql: ^0.12.4 and gql: (without specifying a version number) and still got the same error.

@inc16sec
Copy link

Looks like there's is a problem withgql_code_gen.

@tlvenn
Copy link

tlvenn commented Mar 31, 2021

I am using this override monstrosity to make it works:

dependency_overrides:
  freezed_annotation: 0.14.1
  graphql: 5.0.0-nullsafety.2

  source_gen: ^1.0.0
  glob: ^2.0.0
  dart_style: ^2.0.0
  build: ^2.0.0
  http: ^0.13.0
  http_parser: ^4.0.0
  yaml: ^3.0.0
  # gql: ^0.13.0-nullsafety.2
  gql: ^0.12.4
  gql_exec: ^0.3.0-nullsafety.1
  gql_link: ^0.4.0-nullsafety.2
  gql_http_link: ^0.4.0-nullsafety.1
  gql_dedupe_link: ^2.0.0-nullsafety.1
  gql_build: ^0.2.0-nullsafety.0
  gql_code_builder: ^0.2.0-nullsafety.0
  built_collection: ^5.0.0
  recase: ^4.0.0-nullsafety.0
  graphql_flutter: 5.0.0-nullsafety.1

@inc16sec
Copy link

I am using this override monstrosity to make it works:

dependency_overrides:
  freezed_annotation: 0.14.1
  graphql: 5.0.0-nullsafety.2

  source_gen: ^1.0.0
  glob: ^2.0.0
  dart_style: ^2.0.0
  build: ^2.0.0
  http: ^0.13.0
  http_parser: ^4.0.0
  yaml: ^3.0.0
  # gql: ^0.13.0-nullsafety.2
  gql: ^0.12.4
  gql_exec: ^0.3.0-nullsafety.1
  gql_link: ^0.4.0-nullsafety.2
  gql_http_link: ^0.4.0-nullsafety.1
  gql_dedupe_link: ^2.0.0-nullsafety.1
  gql_build: ^0.2.0-nullsafety.0
  gql_code_builder: ^0.2.0-nullsafety.0
  built_collection: ^5.0.0
  recase: ^4.0.0-nullsafety.0
  graphql_flutter: 5.0.0-nullsafety.1

I can confirm, after overriding the packages you've. provided the builder seemed to work again just fine.
Wondering how?

@featzima
Copy link

With the latest commit it works without override section in our projects

@jlnrrg
Copy link

jlnrrg commented Apr 1, 2021

Thanks for the fast support 😄
I think this thread will now convert to point out issues 🤕
Here I start:

replace '@required' with 'required'

Why is this an issue?

bc required is a new keyword

lib/model/service/graphql/graphql_api.graphql.dart:95:28: Error: The parameter 'min' can't have a value of 'null' because of its type 'int', but the implicit default value is 'null'.
Try adding either an explicit non-'null' default value or the 'required' modifier.
  IntRange({@required this.min, @required this.max});
                           ^^^

@comigor
Copy link
Owner

comigor commented Apr 8, 2021

Hello everyone, we've just merged #283 with NNBD support for Artemis (thank you so much @vasilich6107!)

It'll be released as 7.0.0-beta.2. Test it and please report anything out of place!

Also thanks for your help @glynskyi, but it was easier to migrate without fixing those conflicts.
I'm closing this PR but feel free to open an issue.

@comigor comigor closed this Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.