-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Closed
flutter/engine
#50557Labels
P1High-priority issues at the top of the work listHigh-priority issues at the top of the work listc: API breakBackwards-incompatible API changesBackwards-incompatible API changesc: proposalA detailed proposal for a change to FlutterA detailed proposal for a change to Flutterengineflutter/engine related. See also e: labels.flutter/engine related. See also e: labels.p: requires breaking changeA change that should be batched into the next breaking change to this packageA change that should be batched into the next breaking change to this packageteam-engineOwned by Engine teamOwned by Engine teamtriaged-engineTriaged by Engine teamTriaged by Engine team
Description
Paint (https://api.flutter.dev/flutter/dart-ui/Paint-class.html) is a mutable, framework-specific (i.e. Flutter) "builder" type API.
For example:
void example() {
Paint paint = Paint();
if (paintTheTownRed) {
paint.color = Colors.red;
}
/* ... */
}For historic reasons, Paint is "open" (it can be implemented, extended, or I believe even mixed-in).
Dart added class modifiers in Dart 3.x, which we could utilize:
- class Paint {
+ final class Paint {Of course this would be a breaking change with no trivial workarounds (other than "stop doing that").
Principles
Here is why I think we should make this change, independent of existing usages:
- The
PaintAPI was never intended to be user-extendable (the implementation is not versioned, for example) - As a mutable object, there is no (good) reason to mock or stub the API - just create a "real" object and mutate it
- A
finalclass would let us more easily add new methods (or change signatures over time)
For example #142871 becomes trivial if the class was final.
Existing Code
- Within
org:flutter: ~0, well 1 archived repository. - Github-wide: ~14. Breakdown below.
- Internal to Google: I could not find any.
| repository | reference | how easy to fix |
|---|---|---|
hydro-sdk |
paint.dart |
🔴 hard |
flutter_json_widgets |
paint.freezed.dart |
⚪ false positive (generic bounds) |
flutter_figma-styles-exporter |
fill.dart |
⚪ over 5+ years old |
design_patterns |
decorator_mode.dart |
⚪ over 5+ years old |
bacum |
vacuum_paint.dart |
🟢 appears ~unused |
flutter_painter |
painter_test.dart |
🟢 2+ years old, test could use real implementation. |
flumemo |
pen.dart |
⚪ over 5+ years old and archived. |
blist |
padded_stadium_border_test.dart |
🟢 test, could just us real implementation. |
In conclusion: I propose to make this breaking change.
Metadata
Metadata
Assignees
Labels
P1High-priority issues at the top of the work listHigh-priority issues at the top of the work listc: API breakBackwards-incompatible API changesBackwards-incompatible API changesc: proposalA detailed proposal for a change to FlutterA detailed proposal for a change to Flutterengineflutter/engine related. See also e: labels.flutter/engine related. See also e: labels.p: requires breaking changeA change that should be batched into the next breaking change to this packageA change that should be batched into the next breaking change to this packageteam-engineOwned by Engine teamOwned by Engine teamtriaged-engineTriaged by Engine teamTriaged by Engine team