Review comment for #30

Thanks for writing this up. I agree with the underlying problems, but I think the document needs a structural rewrite. Comments are grouped below.

1. Split into separate RFCs

I'd suggest splitting this into three RFCs:

1. @internal / package-private APIs.
2. Public enum conventions.
3. Experimental APIs and feature flags (@experimental, _features.dart, the windowing APIs).

I see the common theme, which is keeping APIs evolvable without going through the breaking change process. But the decisions can be made separately. The enum conventions are independent of the other two. The experimental RFC would build on @internal, but only in one direction: @internal stands on its own, so there's no reason for it to wait on experimental APIs. They're also at very different maturity levels. The @internal part mostly codifies existing practice, while the enum and experimental parts raise larger questions. Since an RFC is accepted or rejected as a whole, bundling them means the more settled parts wait on the most contested one. The audiences differ as well: @internal mainly concerns contributors and repo layout, while the enum conventions concern API design for every Flutter user.

Experimental APIs deserve their own RFC because they raise questions the other two don't: exempting public APIs from the breaking change policy; how @experimental (a static experimental_member_use warning) relates to the existing runtime gate in _features.dart (driven by the tool's feature flags, so experimental APIs can't be used on stable at all), and whether that gate is still needed; and @internal instance members on public, extensible types (WidgetsBinding.windowingOwner), which the proposed placement rules would themselves forbid. Given the dependency, it makes sense to settle @internal first.

2. @internal / package-private APIs

Structure: two proposals with different motivations

I think the argument should be restructured into two proposals:

1. Drop the requirement that every file in lib/src be exported, and define anything not exported through a barrel file as outside the public API contract. The motivation is maintainability (see below). This stands on its own, and the breaking change exemption should be tied to this rule ("not exported"), not to the annotation. Otherwise it's unclear whether an unexported symbol that someone forgot to annotate is part of the contract.
2. Adopt @internal to make that rule tool-enforced. The motivation is catching accidental exports, and warning consumers who import lib/src directly (invalid_use_of_internal_member). This depends on (1).

Motivation

The current Context assumes package-private APIs are already desirable and only argues for adding a guardrail. It doesn't explain why we need them in the first place.

The core problem, as I see it: since every non-underscored symbol in lib/src can be imported, the current style guide leaves only two options for something we don't want to keep backward compatible. Either make it library-private with _, which forces all code that uses it into a single file and hurts maintainability, or make it public, which commits us to backward compatibility for what is really an implementation detail. The style guide's rationale is that forcing things public leads to better design, but in practice that cost discourages splitting files and refactoring. The RFC should make this argument explicitly and address that rationale directly (see also flutter/flutter#108632).

The background should also mention part/part of and why it's not a viable alternative: it makes every underscored symbol visible across all part files, which removes file-level encapsulation and hurts maintainability even more.

It's also worth noting this matches existing practice. lib/src imports are not considered supported across the Dart ecosystem (implementation_imports), and flutter/packages already uses @internal with hide in the way proposed here (e.g. go_router hides GoRouterStateRegistry from its barrel export).

Existing _-prefixed files and the filename rule

The background should explain what the existing _-prefixed files in lib/src are for. Most of them (_*_io.dart / _*_web.dart) are conditional-import implementations and are unexported for structural reasons, not because of API stability. The windowing files (_window*.dart, _features.dart) are experimental APIs and belong in the experimental RFC. That leaves _accessibility_evaluations.dart as the clearest existing framework example for this RFC: an implementation detail shared within the widgets layer (used by binding.dart), with no reason to be public.

With @internal as the source of truth, I don't think a _ filename should be required for internal code. The conditional-import convention is out of scope and can stay as it is.

@internal vs. @visibleForTesting

I'd suggest keeping @internal and @visibleForTesting semantically distinct rather than letting @internal cover test access. @internal means "not part of the public contract, shared within the package"; @visibleForTesting means "would be private, exposed only for tests". Concretely: @internal symbols should not be accessed from unit tests (tests should exercise them through public behavior), and @visibleForTesting remains the way to grant test-only access, which the analyzer already enforces.

Note that combining the two annotations doesn't express "usable from other files and from tests", because the analyzer treats them as an intersection. I think that's acceptable, but the RFC should state it explicitly.

This makes the open question about @visibleForTesting part of the core proposal rather than a side question, since the two annotations need a clearly defined division of responsibility.

Layers

With experimental APIs moved out, the only current cross-layer use of an unexported file (_features.dart) goes with them. This RFC can then simply state that @internal doesn't change the cross-layer import rule, and leave feature flags to the experimental RFC.

Scope

I'd suggest explicitly scoping this to both the framework and flutter/packages. flutter/packages already follows this in practice, so this would codify an existing unwritten rule rather than introduce a new one. It would also supersede flutter/flutter#108632.

Customer tests

The breaking change policy is defined in terms of the customer test registry, so the exemption needs to be reconciled with it. If a registered test imports package:flutter/src/... and uses a symbol that later becomes @internal, it will fail: most registered suites run flutter analyze --no-fatal-infos, and invalid_use_of_internal_member is a warning, not an info. By the current definition, that would make the change breaking.

I'd suggest:

• Auditing the current registry for such dependencies. A quick search suggests that direct package:flutter/src/ imports are rare, but the flutter_packages suite (material_ui, cupertino_ui tests) has some, and DevTools references framework src library URIs as strings for VM service evaluation, which an import scan wouldn't catch.
• Stating in the registry's criteria that depending on package:flutter/src/ is unsupported, and adding a check when tests are added or updated.
• Stating in the policy that breakages caused by such dependencies are not considered breaking changes.

Enforcement

For enforcement, I'd prefer an analyzer plugin over checks in dev/bots/analyze.dart, so the rules work in the IDE and cover flutter/packages as well. flutter/flutter already has dev/flutter_analyzer_plugin for this kind of migration. Two things the RFC should address:

• Per-library checks (tests not referencing @internal symbols, the instance-member rule) fit the plugin model well. The package-wide check (every public symbol in lib/src is either exported or @internal) needs a view of the whole package's export graph; it's worth confirming whether the plugin API supports that or whether it needs separate tooling.
• flutter_analyzer_plugin is unpublished and flutter/flutter-specific. Sharing these rules with flutter/packages would require extracting them into a published plugin.

Smaller technical points

• The diagnostic for exporting an @internal declaration is invalid_export_of_internal_element, not invalid_internal_annotation (which is about where the annotation may be applied).
• That diagnostic only fires for exports from public libraries, so requiring hide on intra-src/ re-exports is unnecessary.
• The instance-member rule only works for new classes, since adding base/final to an existing class is itself breaking. And for base classes, a new @internal member can still conflict with a member of the same name in a user subclass, so only final/sealed really keep changes non-breaking.

3. Public enum conventions

This isn't a breaking change policy change

I don't think this part changes the breaking change policy at all, so I'd drop the 030-breaking-changes framing from it.

Adding a value to an enum breaks exhaustive switches. That's enforced by the compiler, and the policy can't change it. Adding a static const field to a final class with a private constructor isn't breaking, the same as adding any other static member, so it never needed the breaking change process. What this section really proposes is an API design guideline: decide whether a type is semantically closed before reaching for enum.

Background

The background should reference dart-lang/sdk#63678, where non-exhaustive enums were requested at the language level. The Dart team's position (comment) is that this is a substantial feature and unlikely to be prioritized in the near future. That's the main reason a library-level convention is needed now.

The criterion

I'd suggest the criterion be whether the set of values is closed by nature:

• Closed: the values are complete by definition within Flutter's own model, e.g. Axis (horizontal/vertical) or VerticalDirection (up/down). Use an enum.
• Open: the values mirror something outside Flutter's control that keeps evolving, such as platforms, OS lifecycle states, hardware keys, image formats, or platform input types. Use an enum-like class.

When it's unclear, I'd lean toward open. The costs are asymmetric: if an open set is wrongly declared as an enum, every addition is a breaking change; if a closed set is wrongly declared as a class, consumers lose compile-time exhaustiveness for switch expressions, but switch statements are still covered by the exhaustive_cases lint as long as the class has only private constructors.

The criterion in the RFC, "whether consumers must handle every value exhaustively, rather than whether new values might be added later", doesn't work as a design rule because it's circular: how consumers handle the values is determined by our choice. If we declare an enum, the tools push developers toward exhaustive switches; if we use a class, switch expressions require a fallback.

dart-lang/sdk#63678 also discusses a related criterion (comment): whether each value selects a distinct, non-interchangeable behavior, so that an unknown value can't be handled meaningfully. I don't think that works as a type-level criterion either, since it depends on the use site rather than the type. Keyboard keys each mean something different, yet the set is clearly open and most consumers simply ignore keys they don't handle. Whether a set is open is the only property that belongs to the type itself.

The pattern itself

I don't think a private constructor should be mandatory. The cited precedents show why: LogicalKeyboardKey has a public constructor because hardware can produce key IDs Flutter doesn't know about, and TextInputType.numberWithOptions lets callers combine options. Both are legitimate designs. A rough guide could be: use a public constructor when values outside the predefined set are meaningful on their own (arbitrary key IDs), and a private one when a value only makes sense if Flutter supports it (a user-created TargetPlatform would be meaningless). But this should be left to the API author case by case, and the guideline should describe both variants.

Note that this choice affects tooling: the exhaustive_cases lint only treats a class as enum-like if all its constructors are private, so with a public constructor, switch statements no longer get the missing-case check.

dart-lang/sdk#63678 also discusses alternatives the RFC should evaluate, such as an abstract final class implementing Enum backed by a private enum (comment), which keeps Enum-based APIs working. It also covers a pitfall of the plain pattern: const canonicalization collapses values that don't carry distinct state, so the guideline should require a distinguishing field such as name.

Minor: adding a static const to an enum-like class isn't a compile-time break, but with a private constructor, exhaustive_cases (in the recommended lint set) will flag switch statements that don't cover the new constant. That's comparable to a deprecation warning and shouldn't need the breaking change process, but a sentence in the guideline would help.

Scope

I'd suggest keeping this RFC scoped to a best-practice recommendation for new APIs, and leaving migration of existing enums to separate, case-by-case discussions (one natural opportunity is when a value needs to be added anyway, since that's already breaking).

My main concern is that allowing non-exported code will result in a proliferation of private files, and that users will import them by path regardless of what the styleguide says

I consider more private files are probably better, if we uses it when it fits. If we force everything to be public, we run into two problems

1. discourage us to make change to class and refactoring code for the better.
2. if we ignore (1) and just pumping out breaking change when we want to, the customer still gets broken as frequent as after the proposal. The proposals are not making things worse.

At least with "@internal" people will be warned to stay away from them.

This could result in even more time spent dealing with breaking changes than before. Even though we state that we don't consider these to be breaking changes, usage by g3 customers and customer_tests can still disrupt us.

I don't think so, for g3, they will get warning when they try to use the class, it is not any worse than if anything is forced to be public. for customer_tests, we can reject tests that use direct import.

I think we will deal with breaking changes less.

Are we using any files that are not exported today? Besides feature-flagged experimental APIs like desktop multiwindow.

I don't think so , they are currently all feature flagged based and desktop window related.

Are these rules going to be enforced by our linter or analyzer? Like enforcing that things marked @internal are not imported from outside the library.

we can add lint to flutter_lint if they are not already in there.

Do you have anything in mind for where you want to start using internal code?

system like Focus Management exposed a bunch of stuff like FocusAttachment
or FocusTraversalPolicy.invalidateScopeData
which is more of implementation details. We are probably not going retroactively change them, but for new system we can use package private.

Hi @dkwingsmt
I will separate enum to its own doc and continue the discussion there.

I only talked about visibleForTesting and experimental in open question because I think they serve a slightly different purposes and I am not too sure whether we want to adopt/change them in styleguide. just put it out in open question to see if the discussion lead us toward that direction.

will revise the doc a bit based on suggestion