[tool] Support FeatureFlags.isEnabled in google3 #171797

loic-sharma · 2025-07-08T18:36:53Z

Currently, google3's implementation of FeatureFlags.isEnabled throws. Instead, you have to call FeatureFlags.isFooEnabled, which in google3 returns hardcoded values (google3 does not allow for features to be enabled through configs).

This provides an implementation of FeatureFlags.isEnabled that returns the hardcoded values.

Part of #167668

Alternatives considered

Since this implementation is used by google3, I considered downstreaming this logic to google3. However, this would be brittle: whenever we add a new feature flag, we would need to remember to update google3's FeatureFlags.isEnabled implementation. And since we need to roll the new flag to google3 before we can update google3's FeatureFlags.isEnabled implementation, we can't write a google3 test that verifies that all flags are supported by google3's FeatureFlags.isEnabled.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

loic-sharma · 2025-07-08T18:47:32Z

packages/flutter_tools/lib/src/features.dart

  }

  /// All Flutter feature flags that are enabled.
-  // This member is overriden in google3.


Originally my plan was to override this in google3 with something like:

@override Iterable<Feature> get allEnabledFeatures { return <Feature>[ if (isWebEnabled) flutterWebFeature, if (isLinuxEnabled) flutterLinuxDesktopFeature, ... ]; }

However, I reconsidered as this would be brittle: when we add a few flag, we'd need to remember to update google3's FeatureFlags.allEnabledFeatures implementation. With this new approach, google3's FeatureFlags.isEnabled and FeatureFlags.allEnabledFeatures will just work without any overrides.

matanlurey · 2025-07-08T21:10:41Z

packages/flutter_tools/lib/src/features.dart

+  ///
+  /// This default implementation is used by google3 to facilitate rolls.
+  /// In Flutter, this is overriden by [FlutterFeatureFlags.isEnabled].
+  bool isEnabled(Feature feature) {


I understand why we are doing this, but it's unfortunate because it further increases the cost of new feature flags.

Could you explain why => false isn't a good enough default implementation, or why we need a default implementation at all?

If we went with => false approach, here's the process we'd need to do to add a new feature flag:

Add feature flag to the tool

Wait until this rolls to google3

Update google3 to also add the feature flag to the tool (see this)

We'd be very likely to forget that third step.

With the approach in this PR, all of the work to add a feature flag is centralized in the Flutter tool. google3 wouldn't need any updates to add the new feature flag.

Can you expand on that - why isn't the default of false ok? I think it's going to be pretty rare that, for something that deserves to be flagged, we don't want to explicitly (and separately) turn it on in google3.

Couldn't we make Feature an enum and drop the default case in this switch? That would cause FRoB to fail without a G3Fix since this switch wouldn't be exhaustive if we add a new value to Feature.

That's assuming that we move this implementation of isEnabled into google3.

A feature flag is very likely to require additional work in google3. Even if we make the flutter_tools believe that a certain feature flag is enabled, it might not actually work.

If we went with the default of false here, it would allow us to be in weird middle states. For example:

I add fooFeature and FeatureFlags.isFooEnabled to the Flutter tool and make it on by default.

This rolls into google3. At this point, google3's FeatureFlags.isFooEnabled is true but FeatureFlags.allEnabledFeatures excludes fooFeature.

To fix this, I have to remember to update google3's FeatureFlags.allEnabledFeatures override to do the right thing:

@override Iterable<Feature> get allEnabledFeatures { return <Feature>[ if (isWebEnabled) flutterWebFeature, if (isLinuxEnabled) flutterLinuxDesktopFeature, ... + if (isFooEnabled), fooFeature, ]; }

IMO, this is error-prone. Folks will forget to update google3's feature flags, which will cause weird behaviors in google3.

This problem is avoided by this PR's approach.

IMO, this is error-prone. Folks will forget to update google3's feature flags, which will cause weird behaviors in google3.

They won't forget if Feature is an enum and it breaks compilation when we no longer exhaustively check each value 😄

Oh sorry, I didn't mean to ignore folks' comments in this thread. Y'all replied after I opened the tab, I didn't see your replies until I refreshed the page 😅

Couldn't we make Feature an enum and drop the default case in this switch? That would cause FRoB to fail without a G3Fix since this switch wouldn't be exhaustive if we add a new value to Feature.

I think this alternative solution would work too, but personally I'd prefer a solution that avoids g3fixes. That said, I'm happy to switch to that approach if everyone else prefers it.

No problem! GitHub is being weird and not refreshing comments automatically for me.

When it comes to experiments in google3, it's probably worth forcing whoever is creating a new feature to add the G3Fix so it can be deliberately enabled or disabled for google3. I'd also rather keep things simpler in open-source land and let google3 handle its own special cases in general, but that's just my gut feeling... 😃

chingjun · 2025-07-08T21:57:36Z

packages/flutter_tools/lib/src/features.dart

+  ///
+  /// This default implementation is used by google3 to facilitate rolls.
+  /// In Flutter, this is overriden by [FlutterFeatureFlags.isEnabled].
+  bool isEnabled(Feature feature) {


A feature flag is very likely to require additional work in google3. Even if we make the flutter_tools believe that a certain feature flag is enabled, it might not actually work.

chingjun · 2025-07-08T21:58:01Z

packages/flutter_tools/lib/src/features.dart

+  /// In Flutter, this is overriden by [FlutterFeatureFlags.isEnabled].
+  bool isEnabled(Feature feature) {
+    return switch (feature) {
+      flutterWebFeature => isWebEnabled,


In https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/flutter_features.dart#L24, isWebEnabled is implemented by calling isEnabled. I think it would be better to consistently treat isEnabled as the more lower-level API than is*Enabled.

There's two layers here:

FeatureFlags - This is basically just isFooEnabled properties with static defaults. This is what google3 uses, effectively.

FlutterFeatureFlags - This adds runtime configuration (from env variables and more). This is what the Flutter tool uses. FlutterFeatureFlags replaces FeatureFlags's logic entirely, it just uses FeatureFlags as an interface.

At the FlutterFeatureFlags layer, isEnabled's logic is completely replaced. isEnabled remains the lower-level API than isFooEnabled.

At this FeatureFlags layer, you're right that isEnabled is now a higher-level API than isFooEnabled. However, IMO that's perfectly acceptable: FeatureFlags is just an interface from FlutterFeatureFlags's perspective, it doesn't matter which API is higher or lower level.

Yes I have no doubt that the code will function correctly. But it's still better to be consistent to help the future maintainers to build a better mental model. It's also more error prone.

I could imagine someone being confused, like I did, when they are reading this and wondering how this could work. Especially that there are some amount of code that only exists in google3.

That's why I think we should be consistent in both layers.

loic-sharma · 2025-07-08T22:33:27Z

A feature flag is very likely to require additional work in google3. Even if we make the flutter_tools believe that a certain feature flag is enabled, it might not actually work.

This concern exists today. If you want to control feature flags separately in Flutter versus google3, you'll need to do the same thing before and after this PR: you'll need to update google3 to override FeatureFlags.isFooEnabled.

loic-sharma · 2025-07-09T00:44:36Z

@bkonyi @matanlurey I chatted with @chingjun offline. What would y'all think of making google3 implement FeatureFlags instead of extending it? Anytime you add a new feature flag, google3 will need a g3fix. That's also when you'll need to update google3's isEnabled method to do the right thing.

bkonyi · 2025-07-09T11:36:52Z

@bkonyi @matanlurey I chatted with @chingjun offline. What would y'all think of making google3 implement FeatureFlags instead of extending it? Anytime you add a new feature flag, google3 will need a g3fix. That's also when you'll need to update google3's isEnabled method to do the right thing.

I think that sounds reasonable!

matanlurey · 2025-07-09T16:24:45Z

@bkonyi @matanlurey I chatted with @chingjun offline. What would y'all think of making google3 implement FeatureFlags instead of extending it? Anytime you add a new feature flag, google3 will need a g3fix. That's also when you'll need to update google3's isEnabled method to do the right thing.

I just don't understand why we'd want to add work for something that is going to happen very rarely (wanting to enable a flag specifically for google3 at the exact same time we add it, ostensibly disabled by default, for the external tool).

That being said, this is easy to change. Let's just make a decision, document what it is, and move on.

chingjun

I just don't understand why we'd want to add work for something that is going to happen very rarely (wanting to enable a flag specifically for google3 at the exact same time we add it, ostensibly disabled by default, for the external tool).

The "additional work" is incurred when they add a new feature flag. When someone is adding a new feature flag, we want them to know that the flag doesn't work in google3 as is and probably require additional work.

And the reason we are making this change, is because we intend to make it easier to add new feature flags in the flutter tools, especially runtime feature flags. These do require additional work in google3. It's better to know ahead of time so that they could plan around it.

chingjun · 2025-07-09T18:45:05Z

packages/flutter_tools/lib/src/features.dart

+  /// In Flutter, this is overriden by [FlutterFeatureFlags.isEnabled].
+  bool isEnabled(Feature feature) {
+    return switch (feature) {
+      flutterWebFeature => isWebEnabled,


Yes I have no doubt that the code will function correctly. But it's still better to be consistent to help the future maintainers to build a better mental model. It's also more error prone.

I could imagine someone being confused, like I did, when they are reading this and wondering how this could work. Especially that there are some amount of code that only exists in google3.

That's why I think we should be consistent in both layers.

loic-sharma · 2025-07-10T00:53:22Z

I'll close this in favor of:

[tool] Support FeatureFlags.isEnabled in google3

740efab

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 8, 2025

loic-sharma commented Jul 8, 2025

View reviewed changes

loic-sharma requested review from bkonyi and matanlurey July 8, 2025 21:08

loic-sharma marked this pull request as ready for review July 8, 2025 21:08

loic-sharma requested a review from chingjun July 8, 2025 21:08

matanlurey reviewed Jul 8, 2025

View reviewed changes

chingjun reviewed Jul 8, 2025

View reviewed changes

matanlurey approved these changes Jul 9, 2025

View reviewed changes

chingjun reviewed Jul 9, 2025

View reviewed changes

loic-sharma mentioned this pull request Jul 10, 2025

[tool] Make google3 have to override feature flags #171933

Merged

9 tasks

loic-sharma closed this Jul 10, 2025

[tool] Support FeatureFlags.isEnabled in google3 #171797

[tool] Support FeatureFlags.isEnabled in google3 #171797

Uh oh!

Conversation

loic-sharma commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Alternatives considered

Pre-launch Checklist

Uh oh!

loic-sharma Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkonyi Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma commented Jul 8, 2025

Uh oh!

loic-sharma commented Jul 9, 2025

Uh oh!

bkonyi commented Jul 9, 2025

Uh oh!

matanlurey commented Jul 9, 2025

Uh oh!

chingjun left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loic-sharma commented Jul 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

loic-sharma commented Jul 8, 2025 •

edited

Loading

loic-sharma Jul 8, 2025 •

edited

Loading

bkonyi Jul 8, 2025 •

edited

Loading

loic-sharma Jul 8, 2025 •

edited

Loading

loic-sharma Jul 8, 2025 •

edited

Loading