feat: add RestApi.extra#892
Conversation
|
CI is currently failing on master due to a pre-existing issue: the package_api_docs lint rule was removed in Dart 3.7.0, but it's still referenced in retrofit/analysis_options.yaml, causing dart analyze to exit with code 2. Removing that line should fix the CI. |
|
@huanghui1998hhh Please checke |
There was a problem hiding this comment.
Pull request overview
Adds support for specifying global Dio extra values at the @RestApi(...) level, and updates the code generator/tests to merge these global extras with method-level @Extra(...) values.
Changes:
- Extend
RestApiannotation to include a newextrafield. - Update generator to merge
@RestApi(extra: ...)(lower priority) with method-level@Extra(...)(higher priority). - Add/extend tests to validate default values and global extra generation behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| retrofit/lib/http.dart | Adds RestApi.extra field and documents how global extras merge with method-level extras. |
| retrofit/test/http_test.dart | Extends RestApi default-value test coverage to include headers and extra. |
| generator/lib/src/generator.dart | Implements global RestApi.extra extraction and merge logic in _generateExtra. |
| generator/test/src/generator_test_src.dart | Adds generator expectations for global extras and override/merge cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// API interface. Method-level extra data specified with [@Extra] will be | ||
| /// merged on top of these global extra fields. If the same key exists in | ||
| /// both the global [extra] and a method-level [@Extra], the method-level |
There was a problem hiding this comment.
Dartdoc links generally use [Identifier] references; including @ inside the brackets (e.g., [@Extra]) usually prevents linking/resolution. Consider changing [@Extra] references here to [Extra] (and similarly for other annotation references) so the generated docs link correctly.
| /// API interface. Method-level extra data specified with [@Extra] will be | |
| /// merged on top of these global extra fields. If the same key exists in | |
| /// both the global [extra] and a method-level [@Extra], the method-level | |
| /// API interface. Method-level extra data specified with [Extra] will be | |
| /// merged on top of these global extra fields. If the same key exists in | |
| /// both the global [extra] and a method-level [Extra], the method-level |
| final globalExtraMap = | ||
| clientAnnotationConstantReader?.peek('extra')?.mapValue; | ||
| final globalExtras = globalExtraMap != null | ||
| ? _dartObjectMapToExtraMap(globalExtraMap, m) |
There was a problem hiding this comment.
When converting global @RestApi(extra: ...) values, _dartObjectMapToExtraMap is passed the current MethodElement (m) for error reporting. If the global extra map contains a non-String key, the InvalidGenerationSourceError will incorrectly point at the method instead of the annotated API interface/class, which makes the error harder to understand. Consider passing the ClassElement (e.g., m.enclosingElement) for global extras, and keep m only for method-level @Extra handling.
| ? _dartObjectMapToExtraMap(globalExtraMap, m) | |
| ? _dartObjectMapToExtraMap(globalExtraMap, m.enclosingElement) |
| final _extra = <String, dynamic>{ | ||
| 'platform': 'mobile', | ||
| 'appVersion': '1.0.0', | ||
| }; |
There was a problem hiding this comment.
This @ShouldGenerate expectation uses a multi-line map literal for a 2-entry _extra map. Elsewhere in this file, 1–2 entry extras are asserted as single-line literals (e.g., { 'key': 'value', 'key2': 'value2' }), which is the formatting typically emitted by literalMap and may cause this test to fail or be brittle across formatter/emitter versions. Align the expected snippet with the generator’s actual formatting (likely a single-line map for two entries) to stabilize the test.
| final _extra = <String, dynamic>{ | |
| 'platform': 'mobile', | |
| 'appVersion': '1.0.0', | |
| }; | |
| final _extra = <String, dynamic>{'platform': 'mobile', 'appVersion': '1.0.0'}; |
| @ShouldGenerate( | ||
| ''' | ||
| final _extra = <String, dynamic>{ | ||
| 'platform': 'desktop', | ||
| 'appVersion': '2.0.0', | ||
| }; | ||
| ''', | ||
| contains: true, | ||
| ) |
There was a problem hiding this comment.
This @ShouldGenerate expectation uses a multi-line map literal for a 2-entry _extra map. Given existing expectations in this same file for 2-entry extras are single-line literals, this assertion may not match the generator output and can become brittle across code_builder emitter changes. Update the expected snippet to match the actual generated formatting for two entries.
| @ShouldGenerate( | |
| ''' | |
| final _extra = <String, dynamic>{ | |
| 'platform': 'desktop', | |
| 'appVersion': '2.0.0', | |
| }; | |
| ''', | |
| contains: true, | |
| ) | |
| @ShouldGenerate(''' | |
| final _extra = <String, dynamic>{'platform': 'desktop', 'appVersion': '2.0.0'}; | |
| ''', contains: true) |
As title.