Fix warnings, skip buildScope when not using inflate

pingbird · Mar 25, 2023 · dabe928 · dabe928
1 parent 5e8046f
commit dabe928
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 31 deletions.
diff --git a/boxy/analysis_options.yaml b/boxy/analysis_options.yaml
@@ -62,14 +62,11 @@ linter:
     # - cascade_invocations # doesn't match the typical style of this repo
     - cast_nullable_to_non_nullable
     # - close_sinks # not reliable enough
-    - collection_methods_unrelated_type
-    - combinators_ordering
     # - comment_references # blocked on https://github.com/dart-lang/linter/issues/1142
     - conditional_uri_does_not_exist
     # - constant_identifier_names # needs an opt-out https://github.com/dart-lang/linter/issues/204
     - control_flow_in_finally
     - curly_braces_in_flow_control_structures
-    - dangling_library_doc_comments
     - depend_on_referenced_packages
     - deprecated_consistency
     # - diagnostic_describe_all_properties # enabled only at the framework level (packages/flutter/lib)
@@ -79,18 +76,15 @@ linter:
     - empty_catches
     - empty_constructor_bodies
     - empty_statements
-    - enable_null_safety
     - eol_at_end_of_file
     - exhaustive_cases
     - file_names
     - flutter_style_todos
     - hash_and_equals
     - implementation_imports
-    - implicit_call_tearoffs
     - iterable_contains_unrelated_type
     # - join_return_with_assignment # not required by flutter style
     - leading_newlines_in_multiline_strings
-    - library_annotations
     - library_names
     - library_prefixes
     - library_private_types_in_public_api
@@ -183,7 +177,6 @@ linter:
     - unnecessary_getters_setters
     # - unnecessary_lambdas # has false positives: https://github.com/dart-lang/linter/issues/498
     - unnecessary_late
-    - unnecessary_library_directive
     - unnecessary_new
     - unnecessary_null_aware_assignments
     - unnecessary_null_aware_operator_on_extension_on_nullable
@@ -198,7 +191,6 @@ linter:
     - unnecessary_string_interpolations
     - unnecessary_this
     - unnecessary_to_list_in_spreads
-    - unreachable_from_main
     - unrelated_type_equality_checks
     - unsafe_html
     - use_build_context_synchronously
@@ -216,7 +208,6 @@ linter:
     - use_rethrow_when_possible
     # - use_setters_to_change_properties # boxy: preference
     # - use_string_buffers # has false positives: https://github.com/dart-lang/sdk/issues/34182
-    - use_string_in_part_of_directives
     - use_super_parameters
     - use_test_throws_matchers
     # - use_to_and_as_if_applicable # has false positives, so we prefer to catch this by code-review

diff --git a/boxy/example/lib/pages/line_numbers.dart b/boxy/example/lib/pages/line_numbers.dart
@@ -63,7 +63,7 @@ class LineNumberPageState extends State<LineNumberPage> {
           ),
         ),
       ),
-      numberBg: Container(color: palette.primary),
+      numberBg: ColoredBox(color: palette.primary),
       numberAlignment: numberAlignment,
     );
 

diff --git a/boxy/example/lib/pages/product_tile_simple.dart b/boxy/example/lib/pages/product_tile_simple.dart
@@ -101,7 +101,7 @@ class SeebTitleState extends State<SeebTitle> {
     return ClipRRect(
       borderRadius: BorderRadius.circular(8),
       child: Stack(children: [
-        Positioned.fill(child: Container(color: const Color(0xFFC7CEEA))),
+        const Positioned.fill(child: ColoredBox(color: Color(0xFFC7CEEA))),
         AnimatedContainer(
           duration: const Duration(milliseconds: 500),
           width: expanded ? 450 : 350,

diff --git a/boxy/lib/src/boxy/inflating_element.dart b/boxy/lib/src/boxy/inflating_element.dart
@@ -139,6 +139,8 @@ mixin InflatingRenderObjectMixin<
   InflatingElement? _context;
   _InflationCallback? _inflater;
   var _indexedChildCount = 0;
+  var _needsBuildScope = false;
+  var _didInflate = false;
 
   /// The current element that manages this RenderObject.
   ///
@@ -187,6 +189,10 @@ mixin InflatingRenderObjectMixin<
     // This is a symptom of us not understanding how GlobalKeys work, and/or a
     // bug in the framework, but we do have exhaustive testing that ensures
     // nothing is broken *too* badly in this regard.
+    if (_inflateQueue.isEmpty && !_needsBuildScope) {
+      return;
+    }
+    _needsBuildScope = false;
     _allowSubtreeMutation(() {
       context.owner!.buildScope(context, () {
         for (final child in _inflateQueue) {
@@ -220,6 +226,7 @@ mixin InflatingRenderObjectMixin<
     _inflateQueue.add(child as ChildHandleType);
     childHandles.add(child);
     childHandleMap[id] = child;
+    _didInflate = true;
 
     return child;
   }
@@ -345,10 +352,12 @@ mixin InflatingRenderObjectMixin<
     updateChildHandles(doingLayout: true);
     context._wrapInflater((inflater) {
       _inflater = inflater;
+      _didInflate = false;
       try {
         performInflatingLayout();
       } finally {
         flushInflateQueue();
+        _needsBuildScope = _didInflate;
         _inflater = null;
       }
     });

diff --git a/boxy/test/build_scope_test.dart b/boxy/test/build_scope_test.dart
@@ -7,26 +7,36 @@ class ProxyBoxy extends BoxyDelegate {}
 void main() {
   // Viewports call buildScope, which can fail if we call layout inside of our
   // own buildScope for inflation.
-  testWidgets(
-      'CustomScrollView smoketest',
-      (tester) => tester.runAsync(() async {
-            await tester.pumpWidget(MaterialApp(
-              home: CustomBoxy(
-                delegate: ProxyBoxy(),
-                children: [
-                  BoxyId(
-                      id: #list,
-                      child: CustomScrollView(
-                        slivers: [
-                          SliverList(
-                            delegate: SliverChildListDelegate([
-                              for (int i = 0; i < 10; i++) Text('$i'),
-                            ]),
-                          ),
-                        ],
-                      )),
+  testWidgets('CustomScrollView child smoketest', (tester) async {
+    await tester.pumpWidget(
+      MaterialApp(
+        home: CustomBoxy(
+          delegate: ProxyBoxy(),
+          children: [
+            CustomScrollView(
+              slivers: [
+                SliverList(
+                  delegate: SliverChildListDelegate([
+                    for (int i = 0; i < 10; i++) Text('$i'),
+                  ]),
+                ),
+              ],
+            ),
+            BoxyId(
+              id: #namedList,
+              child: CustomScrollView(
+                slivers: [
+                  SliverList(
+                    delegate: SliverChildListDelegate([
+                      for (int i = 0; i < 10; i++) Text('$i'),
+                    ]),
+                  ),
                 ],
               ),
-            ));
-          }));
+            ),
+          ],
+        ),
+      ),
+    );
+  });
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -63,7 +63,7 @@ class LineNumberPageState extends State<LineNumberPage> { @@
               ),
             ),
           ),
-          numberBg: Container(color: palette.primary),
+          numberBg: ColoredBox(color: palette.primary),
           numberAlignment: numberAlignment,
         );
@@ Expand Down @@