[chunhtai]

Proposing change to style guide around package private in public APIs

Tracking issue: flutter/flutter#193960

This proposal directly contradicts flutter/flutter#108632

Pre-launch Checklist

• I read RFC 000.0001: Taxonomy and followed the file and path naming and metadata standards.
• I read RFC 000.0002: Process and confirmed this proposal meets the threshold for a full RFC.
• I read and agree to the Code of Conduct.
• I signed the CLA.
• I have linked an issue from flutter/flutter with the label design doc.
• All existing and new tests are passing.
• I have enabled "Allow edits from maintainers" on this PR so the bot can automatically assign an RFC number (or I will run dart run bin/assign_rfc_number.dart locally when instructed).

If you need help, consider asking for advice on the #hackers channel on Discord.

[justinmc]

I'm definitely on board with having recommendations for when to use enum vs. static const.

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. 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 think that's the key question for me, do we think this could create more problems than it solves?

Some other questions:

• 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.
• Are we using any files that are not exported today? Besides feature-flagged experimental APIs like desktop multiwindow.
• Do you have anything in mind for where you want to start using internal code?

[justinmc]

I agree with this. It's good to have an official recommendation for when to use these. This is even more important now that adding an enum will break the flutter/packages autoroller; we can't accidentally sneak in a new enum value under the radar any more.

[justinmc]

Good rule to make this explicit.

[justinmc]

I can't use an API marked @internal in a test, right?

This styleguide rule is one I disagreed with when I first started on the team, but I've since gotten used to it. I want to say that I agree with the recommendation to not kid ourselves about visibleForTesting being effectively public, and so we should keep the rule as-is.

[chunhtai]

I can't use an API marked @internal in a test, right?

Yes they serve different purposes. so I don't think we can replace visibleForTesting with internal

I want to say that I agree with the recommendation to not kid ourselves about visibleForTesting being effectively public, and so we should keep the rule as-is.

did you mean we should state that changing API that is visibleForTesting is not breaking change, and keep the discouragement as is?

[justinmc]

Yes

[justinmc]

We already have feature flags for this, I think no change is needed.

[chunhtai]

The reason I bring this up is that should we abandon feature flag approach and just mark something as @Experiemental and treat the change to the API as non breaking change?

On the other hand, we can also just use @internal here as well.

Also, using feature flag requires a lot of works as well, is it really worth it if we can just document our rule and lint warning against using these cutting-edge API

[loic-sharma]

I'd lean towards avoiding @experimental in the Flutter SDK. While @experimental declares that we reserve the right to change an API, @experimental doesn't actually make the change any less impactful. I'd argue the Flutter SDK is so foundational that the impact of changing an @experimental API would be too high. Furthermore, @experimental APIs are listed during code completions, making them much more visible than @internal APIs.

... should we abandon feature flag approach and just mark something as @Experiemental and treat the change to the API as non breaking change?

In my mind, these serve different use cases:

• API that should only be used by the framework: Use @internal.
• Public API that reserves the right to change: Use @experimental. (Though again, I don't think the Flutter SDK should use @experimental)
• APIs that aren't ready to be used in production: Gate behind a feature flag that cannot be enabled on stable and use @internal.

On the other hand, we can also just use @internal here as well.

Agreed 👍

[justinmc]

What about the proposal affects this?

[chunhtai]

I can think of a few approach.

1. abandon the feature flags and just use internal or experiemental
2. move this to a share library that is not part of the exported package. (will need to look into whether this is possible).

[loic-sharma]

I understand that feature flags are an exception to the "no relative imports across libraries" rule, but is this exception a problem in practice? This exception seems to be OK so far?

[chunhtai]

yup, I think that is ok to keep it as exception, though we should make sure this doesn't encourage more exceptions

[dkwingsmt]

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).

[chunhtai]

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.

[chunhtai]

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

[loic-sharma]

A guidance we should bubble up: your enum-like class must have at least one non-private constructor.

If you forget this, Dart's recommended lint exhaustive_cases will enforce exhaustive checks on your enum-like class: https://dart.dev/tools/linter-rules/exhaustive_cases

[LongCatIsLooong]

nit: sealed classes are as closed / frozen as enums, what about them?

[LongCatIsLooong]

Loic talked about keeping the rules simple / easy to digest and keeping the complicated details in lint rules. It looks pretty doable to turn these rules into an analyzer plugin lint, instead of adding to the style guide (and/or using a simpler version of this in the style guide).

[LongCatIsLooong]

Same with the next section.

[LongCatIsLooong]

nit: is a breaking change -> can be a breaking change, as those changes are not necessarily breaking according to our breaking change policy.

[chunhtai]

There is a big grey area when the customer_test is not broken. It is that we have to use our judgement to decide whether we "think" this will break people. People don't want to treat new enum as breakage, but when combining with exhaustive_cases it almost guaranteed someone will break.

[LongCatIsLooong]

The @visibleForTesting annotation marks a public API so that developers that have not disabled the invalid_use_of_visible_for_testing_member analyzer error get a warning when they use this API outside of a test directory.

This means that the API has to be treated as being public (nothing prevents a developer from using the API even in non-test code), meaning it must be designed to be a public API, it must be documented, it must be tested, etc. At which point, there's really no reason not to just make it a public API. If anything, the use of @visibleForTesting becomes merely a crutch to convince ourselves that it's ok that we're making something public that we should really not have made public.

So rather than rely on @visibleForTesting, consider designing your APIs so that they are directly testable using the public API, without exposing any sensitive internals.

(One exception is combining @visibleForTesting with @protected. The @protected annotation marks a member as one that is intended for subclasses, so it is already a public API and considered as such. The @visibleForTesting annotation in that case merely enables the member to be called directly in tests without having to create a fake subclass and without having to add //ignore pragmas.)

The existing guide seems pretty reasonable especially with the @protected exception (the member probably should be @nonVirtual too), and it doesn't say it's strictly forbidden?

[chunhtai]

enum rfc has been separated out to #32

[loic-sharma]

This looks good to me overall!

(It looks like the RFC review process asks for a design review meeting before GitHub approval)

[LongCatIsLooong]

Do we have examples where implementation details / secret states have to be exposed within a layer but not to other layers? The examples listed in the next section don't seem super convincing IMO (see the comment below).

[LongCatIsLooong]

Also flutter/flutter#193960 seems to say another motivation for using @internal is to indicate an API is experimental.

[chunhtai]

Looks like people are generally ok with private files that are under feature flags or cross platform code to use @internal and not exported in barrel export.

I think we still need to decide whether it is ok to have package private to hide internal facing class that are not of the above two categories. Looks like in flutter/packages does this a lot, but flutter/flutter is still following the rule as best as it can. Do we want to go ahead and make lift this restriction? I vote for yes, as is in this proposal, WDYT @loic-sharma @justinmc @dkwingsmt @LongCatIsLooong

[justinmc]

Do you have any specific reasons in mind where you want to start using @internal, or is the motivation for this just the existing violations of the styleguide?

Looks like people are generally ok with private files that are under feature flags or cross platform code to use @internal and not exported in barrel export.

What do you have in mind when you say "cross platform code" here? Or do you mean cross-layer?

Looks like in flutter/packages does this a lot

One example is NoOpPage in go_router, which is probably a good usage.

[justinmc]

+1 to automatic enforcement via analyzer.

Overall I think the recommendation in the styleguide is useful as a guideline, but clearly we don't always follow it. I'm supportive of keeping the general guideline but being open to using @internal when it's worth it, especially if we have automated enforcement.

[chunhtai]

Do you have any specific reasons in mind where you want to start using @internal, or is the motivation for this just the existing violations of the styleguide?

yes, NavigatorState.finalizeRoute and FocusTraversalPolicy. invalidateScopeData are some example that I would have use @internal. They are clean up method are only relevant if you know the internal implementation. People only uses them if they only take a part of the subsystem and build on top. I get that may be use case for these method being public, but we are trading maintainability for this use case.

What do you have in mind when you say "cross platform code" here? Or do you mean cross-layer?

I meant cases like conditional import for web vs io, there are a bunch for example like this one https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/_web_image_web.dart

[LongCatIsLooong]

Is there a different motivating example for introducing @internal? I don't feel the 2 focus related examples are particularly good candidates for @internal.

[chunhtai]

cc @loic-sharma and @justinmc , I added this section let me know what you think

[chunhtai]

I updated the wording to discourage use of package private instead of banning.

[justinmc]

A couple questions and ideas but overall LGTM 👍 . You should try to get approval from the other commenters on this file too before merging (I think that's @LongCatIsLooong, @loic-sharma, @dkwingsmt).

[justinmc]

Consider adding this one:

• @internal APIs must be used internally. An @internal API that is referenced nowhere in the codebase should be removed.

I think this helps distinguish it from @Deprecated as discussed in https://github.com/flutter/rfc/pull/30/changes#r4265258174.

If you agree, do you think it would be possible to enforce this in an automated way?

[justinmc]

I would try to avoid this kind of thing unless it's really necessary. It's probably a sign that finalizeRoute should just be public. But I guess I imagine there could be a situation where we do need it.

[chunhtai]

yup, therefore I phrase it as discourage to use @internal as general.

For this particular case, it is for separating navigator and route files, the alternative will be merging them navigator.dart and route.dart.

[dkwingsmt]

I agree with not allowing @internal on instance members of exported classes, but the @internal extension isn't a good workaround for letting other files in the package use a library-private (_) member of an exported class. A same-file @internal top-level function does the same job, since Dart privacy is library-level. Your Helper example in this thread relies on the same mechanism. Compared with the extension, it doesn't need an exception to the style guide rule against extensions, and it actually triggers invalid_use_of_internal_member for external callers. The extension doesn't, because the analyzer resolves the call to the unannotated extension member rather than to the extension.

Repro (external package importing src/ directly):

// package:lib_pkg/src/route_state.dart
import 'package:meta/meta.dart';

class RouteState {
  void _finalizeRoute() {}
}

@internal
extension RouteStateInternal on RouteState {
  void finalizeRoute() => _finalizeRoute();
}

extension RouteStateInternal2 on RouteState {
  @internal
  void finalizeRoute2() => _finalizeRoute();
}

@internal
void finalizeRoute3(RouteState s) => s._finalizeRoute();

// package:app/main.dart
import 'package:lib_pkg/src/route_state.dart';

void main() {
  final s = RouteState();
  s.finalizeRoute();                     // no diagnostic
  RouteStateInternal(s).finalizeRoute(); // no diagnostic
  s.finalizeRoute2();                    // invalid_use_of_internal_member
  finalizeRoute3(s);                     // invalid_use_of_internal_member
}

Suggestion: recommend the top-level function as the workaround. Whether to allow @internal extensions for other cases (e.g. helpers on types we don't own) is your call, but if they are allowed, @internal must go on each extension member, not only on the extension declaration.

[dkwingsmt]

This comment is not answered yet. There is no need of making an exception of using extension for package privates, as they can be completely replaced by top-level functions. There is zero benefit of using extension except for a beautiful .foo() syntax, which has no value for internal functions.

[justinmc]

2 looks good to me

[dkwingsmt]

I also lean towards option 2. Defining public APIs inside _-prefixed files is counterintuitive: the _ prefix reads as package-internal, so contributors editing those files won't realize they are changing public API. I don't feel strongly, though.

[justinmc]

Can we automatically enforce this?

[chunhtai]

yes, I think so.

[justinmc]

Is there a valid use case for a file exposing both a public declaration and @internal, or should we force users to move the @internal stuff to a separate file?

[chunhtai]

a helper internal class that needs to access library private of a public class.

class PublicClass {
  final int _internal;
}

@internal
class Helper {
   void print(PublicClasss a) {
     print(a._internal);
   }
}

[justinmc]

Yeah I guess that might be annoying to split into a separate file.

[justinmc]

Another thing to consider enforcing automatically if we can

[chunhtai]

sg

[dkwingsmt]

I'm generally ok with the current direction, although I found a few non-trivial issues.

[dkwingsmt]

because export ... hide cannot hide class members

This rationale doesn't hold. Exporting a public class that contains an @internal member does not trigger invalid_export_of_internal_element, and external callers who try to call that member through the barrel still get invalid_use_of_internal_member without needing hide.

The real problem is that instance members are part of the class interface, so changing an @internal instance member can still break external subclasses and implementers:

// package:flutter/src/widgets/foo.dart (exported by widgets.dart)
class Foo {
  @internal
  void bar() {}
}

// External package importing package:flutter/widgets.dart:

// 1. Breaks `implements`: external implementers must implement `bar()`,
//    so adding, removing, or changing `bar()` is a breaking change.
class MyFoo1 implements Foo {
  @override
  void bar() {} // Required by the compiler; no @internal warning!
}

// 2. Breaks `extends`: an external subclass can override `bar()` with no
//    warning, or fail to compile if it already has a `bar` with a different
//    signature.
class MyFoo2 extends Foo {
  void bar(int x) {} // Compile error: invalid_override!
}

The earlier base/final/sealed discussion captured this interface problem; I'd use that as the rationale here.

[chunhtai]

that is true, but using internal extension is safer in this case

so the code will be

class Foo {

}

@internal 
class FooInternal {
  void bar() {}
}

this way, unless people go ahead and force import this extension, they won't have access to bar externally, even for override.

[dkwingsmt]

To clarify, I'm not saying that the point doesn't stand. I agree that @internal shouldn't be applied to methods of public classes. I'm suggesting that the RFC should phrase it with a more convincing rationale.

[dkwingsmt]

I agree with not allowing @internal on instance members of exported classes, but the @internal extension isn't a good workaround for letting other files in the package use a library-private (_) member of an exported class. A same-file @internal top-level function does the same job, since Dart privacy is library-level. Your Helper example in this thread relies on the same mechanism. Compared with the extension, it doesn't need an exception to the style guide rule against extensions, and it actually triggers invalid_use_of_internal_member for external callers. The extension doesn't, because the analyzer resolves the call to the unannotated extension member rather than to the extension.

Repro (external package importing src/ directly):

// package:lib_pkg/src/route_state.dart
import 'package:meta/meta.dart';

class RouteState {
  void _finalizeRoute() {}
}

@internal
extension RouteStateInternal on RouteState {
  void finalizeRoute() => _finalizeRoute();
}

extension RouteStateInternal2 on RouteState {
  @internal
  void finalizeRoute2() => _finalizeRoute();
}

@internal
void finalizeRoute3(RouteState s) => s._finalizeRoute();

// package:app/main.dart
import 'package:lib_pkg/src/route_state.dart';

void main() {
  final s = RouteState();
  s.finalizeRoute();                     // no diagnostic
  RouteStateInternal(s).finalizeRoute(); // no diagnostic
  s.finalizeRoute2();                    // invalid_use_of_internal_member
  finalizeRoute3(s);                     // invalid_use_of_internal_member
}

Suggestion: recommend the top-level function as the workaround. Whether to allow @internal extensions for other cases (e.g. helpers on types we don't own) is your call, but if they are allowed, @internal must go on each extension member, not only on the extension declaration.

[dkwingsmt]

I'd suggest allowing @internal on constructors and static members of exported classes. Unlike instance members, they aren't part of the inherited interface—they can't be overridden or required by implements—and the analyzer enforces @internal on them (I verified named constructors, super calls, and static methods). An earlier revision of this RFC allowed this, and it seems to have been dropped without discussion when the instance-member rule was removed.

Allowing @internal on constructors also covers two cases that top-level functions and class modifiers can't handle across files:

• In-package subclassing across files. Suppose we have a public, documented base class Foo and want subclasses in other files of the same layer to extend it, while preventing external packages from extending or instantiating it. Dart's class modifiers can't express package-scoped subclassing (final and sealed block subclassing even from other files in the same package, while base still allows extends outside the package), and a private constructor Foo._() can't be called from another file. A public class with an @internal generative constructor (@internal Foo.internal()) solves this: in-package subclasses call super.internal(), while external subclasses and callers get invalid_use_of_internal_member.
• Const instances in other files. An @internal const constructor (@internal const Foo.internal()) can be used in const expressions in other files of the layer, whereas a top-level helper function wrapping Foo._() cannot produce a compile-time constant.

Static members have no unique capability over same-file top-level functions, but there's no reason to disallow them either, and they keep class-specific helpers namespaced under the class.

[chunhtai]

yeah inheritance is something internal extension can't solve, and a package private constructor can't be solved by extension.

maybe we should just still allow them on the instance member but prefer internal extension when possible. WDYT?

[dkwingsmt]

According to your recent changes I wonder if I was misunderstood. To be clear: My comment above suggested allowing @internal on constructors and static members, not on instance members.

Also, for non-overridable helpers on an exported class, I don't think an @internal extension has more capability than a same-file @internal top-level function (excluded from the barrel with hide) doesn't already do: both access _ members through library privacy, resolve statically, and stay out of the barrel export, while a top-level function avoids carving out an exception to the style guide rule against extensions and needing a custom lint for member annotations. I don't understand the recommendation of extensions in this doc. extensions are basically syntax sugars for top-level functions, which isn't very valuable for internal usages.

[chunhtai]

I added allow on instance member when the class need to be override in the same package, but not outside.

I general I think this is avoidable unless for readability reason. That is why I made the change.

or do you think this is something we shouldn't allow at all?

extensions are basically syntax sugars for top-level functions, which isn't very valuable for internal usages.

that is correct, but sometimes syntax sugar can make the code easier to follow. I am not feeling strongly about this though. If you think we better leave it out for now, I am ok with it

[dkwingsmt]

I added allow on instance member when the class need to be override in the same package, but not outside.

But @internal on a method doesn't restrict overriding at all. The analyzer only flags calls to an @internal member from other packages, so an external subclass can still silently override the method, and changing it can still break that subclass. This rule will not achieve your goal of "overridable in the same package, but not outside".

To achieve your goal, the restriction has to come from the class: external code must be unable to subclass or implement it. This is achievable in the following 3 steps:

1. Make the constructor a named @internal constructor (or private) to block external extends. Subclasses in other files of the package can still call super.internal(), while external ones get invalid_use_of_internal_member. (An unnamed @internal Foo() doesn't work, since implicit super() calls aren't flagged.)
2. Mark the class base to block external implements, which constructors can't prevent.
3. Apply step 1 to every in-package subclass too (or mark it final). Otherwise external code can extend the subclass and override the method with no warning.

If you think we better leave it out for now, I am ok with it

I agree that readability matters, but that applies to private and public APIs alike (if anything, more to public ones). If extensions improve readability, I'd rather see a separate proposal to relax the style guide's rule against extensions across the repo than an exception here only for internal APIs. Therefore I'd suggest leaving extensions out.

[dkwingsmt]

The analyzer does not generally catch @internal types that leak into the public API through exported declarations. invalid_export_of_internal_element only checks declarations exported directly by name, invalid_export_of_internal_element_indirectly only covers a narrow case (an @internal typedef in the signature of an exported top-level function), and library_private_types_in_public_api only checks _-prefixed types. As a result, dart analyze reports nothing on either the framework or the consumer when an @internal type reaches users through a public member signature or supertype:

// lib/src/widgets/foo.dart, exported by lib/widgets.dart with `hide InternalState, InternalBase`
@internal
class InternalState {
  void reset() {}
}

@internal
class InternalBase {
  void helper() {}
}

class PublicController {
  InternalState get state => InternalState();
}

class PublicWidget extends InternalBase {}

// External package, importing only package:flutter/widgets.dart
controller.state.reset();   // no diagnostic
final s = controller.state; // no diagnostic; type inferred
PublicWidget().helper();    // no diagnostic

Leaving this gap open creates three problems:

• External callers get no warning, yet have no stability guarantee. Consumers here neither import src/ nor suppress any warning, and automated checks on flutter/tests won't flag their code either. Yet under this RFC, InternalState and InternalBase are unexported @internal declarations, so renaming reset() or helper() can break valid caller code without a deprecation period.
• API documentation is either broken or misleading. Because InternalState is not exported, dart doc generates no page for it (and emits no warning), leaving PublicController.state with an unlinked return type and no documentation for reset(). Conversely, for PublicWidget, dart doc lists helper() on PublicWidget's page as an ordinary inherited method, giving users every reason to treat it as supported public API.
• It is easy to introduce by accident. A contributor might mark an existing class @internal while a public signature still refers to it, or extract an @internal base class to share implementation across files. Because the Enforcement section implies that the analyzer already catches accidental leaks, reviewers won't know to look for this manually.

I'd suggest adding a rule, enforced by the custom lint: @internal types must not appear in the public signatures or supertypes (extends, implements, with, on) of publicly exported declarations. Instead, either make the type public and keep construction internal with an @internal constructor (see above), or expose it through a public supertype.

[chunhtai]

yes agree. We should definitely have this rule and ci enforcing it

[dkwingsmt]

I also lean towards option 2. Defining public APIs inside _-prefixed files is counterintuitive: the _ prefix reads as package-internal, so contributors editing those files won't realize they are changing public API. I don't feel strongly, though.

[dkwingsmt]

This rule relies on author intent, which can't be verified in review. As raised earlier in this thread, an exported API can be marked @internal (and hidden from the barrel) in one PR, and then deleted in a follow-up PR with no deprecation period. Even when the author genuinely intends to keep the API for internal use, hiding an existing symbol from the barrel breaks external callers immediately without the migration window that @Deprecated provides.

I'd suggest a mechanical rule instead: hiding an already-exported API from the barrel or marking it @internal counts as removing a public API and must go through the standard deprecation process first (deprecate the public API while migrating internal callers to a new private or @internal helper, then remove the deprecated public API after the deprecation period).

[chunhtai]

I'd suggest a mechanical rule instead: hiding an already-exported API from the barrel or marking it @internal counts as removing a public API

It should be treated as breaking change for sure. The loop hole is that once that is done we don't have guideline says when it can be actually removed. so a bad actor can come in mark it internal (with breaking change notice) and remove it immediately after.

The deprecation policy does say we will wait at least a stable release before removing @deprecated API.

We could included that removing @internal API should also wait at least one stable, but then it still require reviewer to catch the bad actor. I felt saying more other than the current statement won't help much.

[dkwingsmt]

That's a good point. Thanks for clarifying. Maybe we should rule that adding @internal to an existing API must be accompanied by a comment of the earliest date that this internal API might be broken, which is 1 year from the PR's landing date.

[chunhtai]

sg

[dkwingsmt]

For the unused-@internal check, references from tests shouldn't count as uses. Test-only access is what @visibleForTesting is for. Consider clarifying whether the "nowhere in this codebase" includes unit tests.

This suggestion also means dropping "or accessed by unit tests" from the permitted cases.

[chunhtai]

sg

[dkwingsmt]

I again suggest keeping experimental APIs out of this RFC, as I mentioned earlier. How experimental APIs should work is more than just a tag, and hasn't been settled in this thread: @internal + feature flag vs. @experimental, how users opt in, how APIs graduate, and whether SDK packages may use them (flutter_test/lib/src/binding.dart already uses the windowing APIs via a file-wide ignore_for_file). I'd suggest removing the experimental category here, and treating the existing windowing APIs and the _features.dart exception as out of scope until a separate RFC covers them.

[chunhtai]

@experimental is a weaker guard than @internal + feature flag. We also already have leaned toward package private + feature flag in our current code, this guideline change only makes it easier to follow for new experimental API. I view it as a net benefit.

[dkwingsmt]

I agree that @internal + feature flag is a stronger guard than @experimental; I'm not arguing for @experimental. My concern is codifying the pattern before its rules are worked out.

The only existing case, windowing, doesn't follow the other rules in this RFC: WidgetsBinding.windowingOwner is an @internal instance member on an exported mixin, and flutter_test uses these APIs through a file-wide ignore_for_file. Once this lands, those become violations to fix or exceptions to carve out. Open questions like how users opt in (importing src/ plus an ignore?) and how an API graduates would get answered by whatever the first few cases happen to do, and changing a published guideline later is harder than getting it right once.

Since windowing is the only experimental API using this pattern today, deferring the category to a separate RFC costs little. If you'd rather keep it here, I'd at least like the RFC to cover how windowing conforms, how users are expected to opt in, and what graduation looks like.

[chunhtai]

I think the current rule in this rfc should have covered this? that the windowingOwner should have been internal extension and should be fixed.

Open questions like how users opt in (importing src/ plus an ignore?)

External package or app should not use it so there is no opt in for them even in their tests. Their only channel is to file an issue to open the API to public

If you meant the test in flutter/flutter, I am less concern about how they get around the @internal decorator. Either they direct import or use ignore_for_file.

[dkwingsmt]

Moving windowingOwner to an internal extension doesn't work technically: WidgetsBinding.createWindowingOwner() has to be overridden by TestWidgetsFlutterBinding in flutter_test (a separate package), and extension methods can't be overridden. And because @internal is strictly package-scoped, flutter_test has no way to touch windowing types without ignore_for_file.

More importantly, "external apps should not use it" contradicts why the API is behind a user-facing --enable-windowing flag in the first place. In fact, the windowingOwner error message, _window.dart doc comments, and examples/multiple_windows all explicitly instruct developers to enable the flag and import src/ (with ignore_for_file) to try it out before it stabilizes (flutter/flutter#177586).

Therefore applying @internal to windowingOwner, even with it converted to an extension, will not solve its problem.

Speaking more broadly, both issues come from the same root cause: @internal is for symbols that are never accessed outside the package, whereas an experimental API is meant to be tried across package boundaries before becoming public. Having an experimental API mechanism is valuable and worth discussing, but it shouldn't be built on @internal, and it involves process questions beyond syntax—such as the criteria and lifecycle for introducing and graduating an experimental API. For this RFC, we should remove the experimental category (leaving windowing as an existing exception for now) and address experimental APIs in a dedicated proposal.

[justinmc]

I wanted to capture two points from the Dash Forum on this RFC:

1. We should be clear in the styleguide that we still encourage APIs that are public and production-quality at all layers, and package private should only be used in rare cases. Ideally we continue to allow users to peel back layers and hack on things, while keeping our maintenance burden and velocity reasonable.
2. As asked by @reidbaker, should we consider breaking up some of our large files? For example basic.dart and framework.dart both are 7,000+ lines. It might be worthwhile to take a look at what it would look like if we did try breaking them up, just for argument's sake.

[Piinks]

should we consider breaking up some of our large files? For example basic.dart and framework.dart both are 7,000+ lines. It might be worthwhile to take a look at what it would look like if we did try breaking them up, just for argument's sake.

I wonder if shorter files could also improve agent performance, maybe worth an eval to see for sure.

[chunhtai]

For example basic.dart and framework.dart both are 7,000+ lines. It might be worthwhile to take a look at what it would look like if we did try breaking them up, just for argument's sake.

I think lines may not be the best indicator but more of a correlation of a file that may be mixing too much class or concept. but to that point these two file may be a good example to break it apart because they are a lump sum of widgets that doesn't relate to the other, and a lot of them can probably be separated without the need of @internal anyway.

I will do a throughly audit on the file once this proposal is approved.

[LongCatIsLooong]

LGTM with a few nits.

[LongCatIsLooong]

which commits the framework to backward compatibility and discourages refactoring.

I feel this would encourage the framework to sit on tech debts instead, if @internal is not counted as tech debts (IIRC there's some sort of tracker for framework tech debts?)

If the API design is leaky and it's a real framework tech debt (the more common case), wouldn't improving the API and deprecating the existing leaky API be the more correct thing to do (especially if you're already refactoring it)?

If the API design is leaky because it has to be leaky (e.g., because of a language limitation), then I feel it's justified to tell API users that the API isn't really a part of the public contract and should not be used (and prevent dartdoc from generating a reference for that API).

NavigatorState.finalizeRoute

I think it's still part of the Route lifecycle contract? A custom Route subclass would still need to call this method in didPop. So this shouldn't really qualify for @internal?

HardwareKeyboard.syncKeyboardState

Not familiar with this one, but Gemini suggested the leak is avoidable if we redesign the API if we pass the initial key presses synchronously. So technically a tech debt?

[LongCatIsLooong]

Should we require a TODO comment for @internal usages for this category, since in most cases they are tech debts?

[chunhtai]

A custom Route subclass would still need to call this method in didPop. So this shouldn't really qualify for @internal?

they can do that by calling super, I think the reason is didpop can have animation, but that is self contained in TransitionRoute. One arguement is someone implement route interface, but it won't work without significant hack in navigator because there is internal logic in base route class.

The only use case I can think of that can't be solve by inheritance is that they implement on top of Route but below TransitionRoute, It will break a bunch of other stuff like gesture pop and predicative back. So I don't think that is something we should support.

I agree this may be able to be prevented through less leaky design, but that will mean we will build transition and animation in mind when designing navigator, or just move TransitionRoute to navigator.dart.

but that back to the question whether we are going to merge classes and different concepts in a single file or not.

Should we require a TODO comment for @internal usages for this category, since in most cases they are tech debts?

I think sometimes this is just unavoidable, and the alternative is big files. but I think requiring a comment reasoning may be a good practice

[dkwingsmt]

This rule contradicts itself on inheritance. A final or sealed class cannot be extended or implemented in any other file. Within the same file, _ declarations already work, so @internal is never needed there; across files, final and sealed forbid the inheritance this bullet tries to permit.

Assuming this rule is an answer to #30 (comment) : Constructors and static members do not need final or sealed. Because they aren't part of the class interface, changing them can't break external extends or implements, and requiring final or sealed needlessly prevents adding them to existing public classes. To allow in-package subclasses across files while blocking external subtypes, use base with a named @internal generative constructor (@internal Foo.internal(); unnamed @internal Foo() misses implicit super() calls).

[dkwingsmt]

Until a stable release that includes the annotation has shipped, changes to the declaration follow the normal breaking change policy.

This does not grant much of a grace period. Since a change on main ships to stable within 1 week to 3 months, the symbol loses protection in at most 3 months, which is much shorter than our deprecation policy (and shorter than the 1-year window we agreed on in #30 (comment)).

What's worse, we cannot keep the symbol exported in the barrel file during this grace period: adding @internal to a top-level declaration requires adding hide to lib/<layer>.dart in the same PR to avoid invalid_export_of_internal_element (and even on class members, invalid_use_of_internal_member is a warning rather than an info). That means external callers using the public barrel are already broken on day 1, and the grace period only protects callers that imported lib/src/ directly.

Combining my suggestions in #30 (comment), the process for making an existing public symbol @internal should be:

• Day 1: Mark the symbol @Deprecated (following the standard deprecation and breaking-change process) while keeping it exported.
• In a stable release at least {1 year for framework, 6 months for packages} later: Replace @Deprecated with @internal and add hide to the barrel export.
  With this process, @internal symbols no longer need a separate grace period or the // This feature was made internal after v<version>. comment convention.

[dkwingsmt]

This comment is not answered yet. There is no need of making an exception of using extension for package privates, as they can be completely replaced by top-level functions. There is zero benefit of using extension except for a beautiful .foo() syntax, which has no value for internal functions.

[dkwingsmt]

I added allow on instance member when the class need to be override in the same package, but not outside.

But @internal on a method doesn't restrict overriding at all. The analyzer only flags calls to an @internal member from other packages, so an external subclass can still silently override the method, and changing it can still break that subclass. This rule will not achieve your goal of "overridable in the same package, but not outside".

To achieve your goal, the restriction has to come from the class: external code must be unable to subclass or implement it. This is achievable in the following 3 steps:

1. Make the constructor a named @internal constructor (or private) to block external extends. Subclasses in other files of the package can still call super.internal(), while external ones get invalid_use_of_internal_member. (An unnamed @internal Foo() doesn't work, since implicit super() calls aren't flagged.)
2. Mark the class base to block external implements, which constructors can't prevent.
3. Apply step 1 to every in-package subclass too (or mark it final). Otherwise external code can extend the subclass and override the method with no warning.

If you think we better leave it out for now, I am ok with it

I agree that readability matters, but that applies to private and public APIs alike (if anything, more to public ones). If extensions improve readability, I'd rather see a separate proposal to relax the style guide's rule against extensions across the repo than an exception here only for internal APIs. Therefore I'd suggest leaving extensions out.

[dkwingsmt]

nit: I wonder if we need to make an exception to generated code, such as those in pigeon, which generates functions for all APIs regardless of whether they're used. cc @tarrinneal

[tarrinneal]

pigeon shouldn't need an exception, but I don't know about the jnigen/ffigen output.

[dkwingsmt]

Any file in lib/src/<layer>/ not exported by lib/<layer>.dart must have a _ filename prefix

As I mentioned in #30 (comment), it's sufficient to use @internal as the single source of truth, and there's no longer a technical need to require a _ filename prefix for unexported files.

While I agree that this convention can be a helpful visual cue, it has a practical cost:

• If an unexported file gains its first public declaration, it has to be renamed to drop the _ before it can be exported.
• If an exported file loses its last public declaration, we either have to rename it with a _ prefix, or keep exporting it with every remaining @internal declaration listed in hide (which this doc allows).

Renaming a file is disruptive: every import of it has to be updated, links to it break, its history is lost from git log without --follow, and it moves around in file listings. That cost isn't justified just to keep the filename in sync with information that @internal already carries. The analyzer and the CI check can work off @internal and the barrel's export list directly, without looking at filenames.

I'd suggest dropping the filename requirement, and keeping the _ prefix only as the existing convention for conditional-import implementation files.

[dkwingsmt]

with lib/src/foundation/_features.dart as the only permitted exception

_features.dart isn't the only cross-layer import of an unexported file today. There are at least two more:

• lib/src/web.dart: a hand-copied subset of package:web (DOM and JS interop types such as HTMLImageElement and document), kept in the framework so that Flutter doesn't depend on package:web. It sits directly in lib/src/, outside any layer directory, and isn't exported by any barrel. It's imported via ../web.dart by web implementations in three layers: foundation/_error_dumper_web.dart, painting/_web_image_info_web.dart and painting/_network_image_web.dart, and widgets/_web_image_web.dart and widgets/_platform_selectable_region_context_menu_web.dart.
• lib/src/painting/_web_image_info_{io,web}.dart: defines WebImageInfo. It isn't exported by painting.dart, but the widgets layer imports it directly: widgets/image.dart (as a conditional import) and widgets/_web_image_{io,web}.dart. It can't simply move to the widgets layer, because it's created in the painting layer (painting/_network_image_web.dart, behind NetworkImage) and consumed in the widgets layer (Image checks for it to switch to an <img> platform view). Making it public isn't trivial either, since its htmlImage getter returns the HTMLImageElement type from the web.dart shim above.

I think this rule is still valuable, since keeping cross-layer "private imports" to a minimum is crucial for layer isolation, and for letting external code build on the lower layers (such as rendering) the same way the framework's own higher layers do. I suggest adding these two as exceptions for now. web.dart should probably be a permanent exception. WebImageInfo might be worth another look in the future, to see whether we can factor it out or turn it into a meaningful public class: today, a third-party image widget built on NetworkImage receives a WebImageInfo on the web but has no way to recognize it, which is exactly the kind of gap this rule is meant to prevent.

[dkwingsmt]

that is not exported by a public barrel file and is annotated with @internal

This phrasing is problematic.

• This condition doesn't cover @internal constructors and members of exported classes, which line 102 allows. They're reachable through the barrel, so they fail "not exported", and changing them would still count as a breaking change, which defeats the point of marking them @internal.
• For top-level declarations, the two halves are equivalent once the analyzer and CI checks are in place: an exported @internal declaration triggers invalid_export_of_internal_element, and the planned checks require unexported declarations to be @internal.
• It doesn't take the grace period into account.

I recommend rephrasing the condition to be "annotated with @internal" alone, which is likely equivalent to what you're describing but more accurate.

[dkwingsmt]

Moving windowingOwner to an internal extension doesn't work technically: WidgetsBinding.createWindowingOwner() has to be overridden by TestWidgetsFlutterBinding in flutter_test (a separate package), and extension methods can't be overridden. And because @internal is strictly package-scoped, flutter_test has no way to touch windowing types without ignore_for_file.

More importantly, "external apps should not use it" contradicts why the API is behind a user-facing --enable-windowing flag in the first place. In fact, the windowingOwner error message, _window.dart doc comments, and examples/multiple_windows all explicitly instruct developers to enable the flag and import src/ (with ignore_for_file) to try it out before it stabilizes (flutter/flutter#177586).

Therefore applying @internal to windowingOwner, even with it converted to an extension, will not solve its problem.

Speaking more broadly, both issues come from the same root cause: @internal is for symbols that are never accessed outside the package, whereas an experimental API is meant to be tried across package boundaries before becoming public. Having an experimental API mechanism is valuable and worth discussing, but it shouldn't be built on @internal, and it involves process questions beyond syntax—such as the criteria and lifecycle for introducing and graduating an experimental API. For this RFC, we should remove the experimental category (leaving windowing as an existing exception for now) and address experimental APIs in a dedicated proposal.