RendererElement Generic Type Argument

[MelbourneDeveloper]

Several methods like 'calculateLabelPosition' of SunburstArcLabelDecorator require downcasting. For example:

 @override
  ArcLabelPosition calculateLabelPosition(
    TextElement labelElement,
    TextStyle labelStyle,
    int insideArcWidth,
    int outsideArcWidth,
    ArcRendererElement<D> arcRendererElement,
    ArcLabelPosition labelPosition,
  ) {
    assert(arcRendererElement is SunburstArcRendererElement);

    if ((arcRendererElement as SunburstArcRendererElement).isOuterMostRing ==
        true) {
      return super.calculateLabelPosition(
        labelElement,
        labelStyle,
        insideArcWidth,
        outsideArcWidth,
        arcRendererElement,
        outerRingArcLabelPosition,
      );
    } else if (arcRendererElement.isLeaf == true) {
      return super.calculateLabelPosition(
        labelElement,
        labelStyle,
        insideArcWidth,
        outsideArcWidth,
        arcRendererElement,
        innerRingLeafArcLabelPosition,
      );
    } else {
      /// TODO: Improve label handling for sunburst chart. When a
      /// more sophisticated collision detection is in place, we can draw the
      /// label for inner arc outside when it doesn't collide with outer arcs.

      // Force label for arc on the inner ring inside.
      return ArcLabelPosition.inside;
    }
  }

You can see that in the above method, there is a runtime assertion that could cause an exception when this would not be necessary with an extra type argument. If we gave an extra type like this:

class SunburstArcLabelDecorator<D>
    extends ArcLabelDecorator<D, SunburstArcRendererElement<D>> { }

/// New base type for renderer elements
abstract class BaseRendererElement<D> {
  Color? color;
  int? index;
  num? key;
  D? domain;
  late ImmutableSeries<D> series;

  void updateAnimationPercent(
    BaseRendererElement<D> previous,
    BaseRendererElement<D> target,
    double animationPercent,
  );

  // BaseRendererElement<D> clone();
}

// ...

// We can do this:

 @override
  ArcLabelPosition calculateLabelPosition(
    TextElement labelElement,
    TextStyle labelStyle,
    int insideArcWidth,
    int outsideArcWidth,
    SunburstArcRendererElement<D> arcRendererElement,
    ArcLabelPosition labelPosition,
  ) {
    if (arcRendererElement.isOuterMostRing ?? false) {
      return super.calculateLabelPosition(
        labelElement,
        labelStyle,
        insideArcWidth,
        outsideArcWidth,
        arcRendererElement,
        outerRingArcLabelPosition,
      );
    } else if (arcRendererElement.isLeaf == true) {
      return super.calculateLabelPosition(
        labelElement,
        labelStyle,
        insideArcWidth,
        outsideArcWidth,
        arcRendererElement,
        innerRingLeafArcLabelPosition,
      );
    } else {
      /// TODO: Improve label handling for sunburst chart. When a
      /// more sophisticated collision detection is in place, we can draw the
      /// label for inner arc outside when it doesn't collide with outer arcs.

      // Force label for arc on the inner ring inside.
      return ArcLabelPosition.inside;
    }
  }

}

This makes the assertion exception impossible in the first place. This means we have to ripple the new type arguments all throughout the common package like this:

class ArcLabelDecorator<D, TBaseRendererElement extends BaseRendererElement<D>>
    extends ArcRendererDecorator<D, TBaseRendererElement> {}

It's a big change, but I don't think it would be possible to do this without affecting the example app, and most other apps won't need to change code unless there are a lot of custom charts.

I have started on this change in the types2 branch. You can see how this change affects the common package in this branch.

Any discussion around why this might not work are welcome, but essentially, this removes a lot of dynamic typing, would allow for better compiler optimizations, and less need for casting and runtime errors.