Feat/compatibilty_with_riverpod3 #43
Conversation
…encies - Resolve compatibility issues with Freezed 3.0.0 and above - Update Dart SDK to >=3.0.0 - Upgrade dependencies to latest versions - Add support for Freezed mixed mode classes - Update generator to handle new Freezed class structures - Improve compatibility with newer analyzer and source_gen versions
- Upgrade cloud_firestore dependency to ^6.0.0 - Update pubspec.yaml and CHANGELOG for versioning - Modify transaction and batch methods to remove 'id' field from documents - Enhance testing setup in README for running integration tests - Add new tests for Freezed and other integration scenarios
…m and generator - Upgrade Dart SDK constraints to >=3.8.0 <4.0.0 in pubspec.yaml files - Update dependencies in example and generator packages to latest versions - Modify integration tests and example code for compatibility with updated dependencies - Refactor code for improved readability and maintainability
…tore_odm and generator - Updated version to 1.3.0 in pubspec.yaml files - Upgraded packages to support analyzer ^8 and set minimum SDK to 3.8.0 - Updated CHANGELOG to reflect changes
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Streamlined expect statements in integration tests for better readability. - Removed unnecessary print statements from collection data and generator files. - Enhanced code clarity by consolidating multi-line statements into single lines where appropriate.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…eodm-flutter into feat/upgrade-analyzer8 merge
- Changed generated file postfix from .g.dart to .firebase.dart for better compatibility with Riverpod. - Updated documentation to reflect the new file naming convention. - Modified example files to include part declarations for .firebase.dart files. - Bumped version to 2.0.0 in pubspec.yaml and updated CHANGELOG to indicate breaking changes.
Summary of ChangesHello @dorklein, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant update to improve compatibility with Riverpod by changing the naming convention for generated files from Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces compatibility with Riverpod 3 by changing the generated file extension to .firebase.dart and updating the build process. This is a significant breaking change, which is well-documented in the CHANGELOG and README files. The dependencies have also been updated accordingly. I've identified a few typos in the documentation and some areas for improvement in the test and example code, including a potentially confusing provider implementation in the new Riverpod integration test. Additionally, there's a manual step for local development that could be made safer.
| Note: make sure in `packages/cloud_firestore_odm_generator/pubspec.yaml` to replace | ||
| ``` | ||
| cloud_firestore_odm: {VERSION} | ||
| ``` | ||
| with | ||
| ``` | ||
| cloud_firestore_odm: | ||
| path: ../cloud_firestore_odm | ||
| ``` | ||
| This is required for the tests to run. Set back to correct version before deploying. |
There was a problem hiding this comment.
This manual step of modifying pubspec.yaml for local testing is error-prone, as a developer might forget to revert the changes before creating a pull request or publishing. This could lead to a broken release with a local path dependency. Consider automating this with a script or using a more robust solution for managing local dependencies in a monorepo, such as Melos, to mitigate this risk.
| _$Person userProfilesCollection(Ref ref) { | ||
| return Person(firstName: 'John', lastName: 'Doe', ignored: 1); | ||
| } |
There was a problem hiding this comment.
The provider userProfilesCollection is misleadingly named. It returns a single Person object, not a collection of user profiles. This could be very confusing for anyone reading this code. Please consider renaming it to reflect what it actually provides, for example, examplePerson.
Person examplePerson(Ref ref) {
return Person(firstName: 'John', lastName: 'Doe', ignored: 1);
}| } | ||
|
|
||
| void main() { | ||
| print('userProfileRef: ${_$Person} - ${userProfilesCollectionProvider}'); |
There was a problem hiding this comment.
The type _$_Person appears to be a typo and refers to a private class generated by freezed, which should not be used directly. You should use the public Person class instead. This will likely cause an analyzer error.
print('userProfileRef: ${Person} - ${userProfilesCollectionProvider}');| - Within the `product.dart` file, apply the part declaration (`part 'product.g.dart'`). | ||
| - Code generation will create a file in the same directory named `product.firebase.dart`. | ||
| - Within the `product.dart` file, apply the part declaration (`part 'product.firebase.dart'`). | ||
| - You should also have a `.g.dart` for the json serilizer (`part 'product.g.dart'`). |
There was a problem hiding this comment.
There's a typo here. It should be serializer.
| - You should also have a `.g.dart` for the json serilizer (`part 'product.g.dart'`). | |
| - You should also have a `.g.dart` for the json serializer (`part 'product.g.dart'`). |
| @@ -1,3 +1,18 @@ | |||
| ## 2.0.0 | |||
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibilty with Riverpod ^3. | |||
There was a problem hiding this comment.
There's a typo here. It should be compatibility.
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibilty with Riverpod ^3. | |
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibility with Riverpod ^3. |
| @@ -1,3 +1,19 @@ | |||
| ## 2.0.0 | |||
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibilty with Riverpod ^3. | |||
There was a problem hiding this comment.
There's a typo here. It should be compatibility.
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibilty with Riverpod ^3. | |
| - **BREAKING**: Must include a "part '<FILENAME>.firebase.dart';" instead of just part "'<FILENAME>.g.dart';". This is because of compatibility with Riverpod ^3. |
| # build_extensions: { ".dart": [".firebase.g.part"] } | ||
| # auto_apply: dependents | ||
| # build_to: cache |
There was a problem hiding this comment.
These commented-out lines appear to be leftovers from refactoring. It would be cleaner to remove them to avoid confusion.
| const SimpleFreezed({required this.a}); | ||
| factory SimpleFreezed.fromJson(Map<String, Object?> json) => _$SimpleFreezedFromJson(json); | ||
|
|
||
| @override |
There was a problem hiding this comment.
The @override annotation is incorrect here because SimpleFreezed does not extend a class that has an a member; it only applies a mixin. Please remove it.
|
|
||
| @override | ||
| final int a; | ||
| Map<String, Object?> toJson() => _$SimpleFreezedToJson(this); |
There was a problem hiding this comment.
This toJson() method is redundant. When using freezed with json_serializable and a fromJson factory constructor, a toJson() method is automatically generated on the implementation class. You can safely remove this manual implementation.
1 participant
Fixes #<issue_number_goes_here>