Skip to content

Avoid using a Key whose only purpose is to be looked up in a test#170952

Merged
auto-submit[bot] merged 5 commits intoflutter:masterfrom
justinmc:no-keys
Jun 26, 2025
Merged

Avoid using a Key whose only purpose is to be looked up in a test#170952
auto-submit[bot] merged 5 commits intoflutter:masterfrom
justinmc:no-keys

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Jun 20, 2025 •
edited
Loading

This PR investigates moving away from the few places where we rely on Keys to instead solely look up a widget in a test. Reasons:

  • No extra stuff in users' apps that they don't need for production.
  • Avoid collisions between Keys in users' apps that might have the same identifier string.

I'm definitely still up for discussion on whether or not this is a good idea!

Written in response to the discussion on #170761 (comment).

@justinmc
Remove the Key in PopupMenItem
e961a37 
There is one more to get in snackbar.dart.
@justinmc justinmc self-assigned this Jun 20, 2025
@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jun 20, 2025
@justinmc justinmc mentioned this pull request Jun 20, 2025
Merged
9 tasks
@AhmedLSayed9
Copy link
Contributor

AhmedLSayed9 commented Jun 21, 2025

This should also be updated at:

flutter/packages/flutter/test/material/popup_menu_test.dart

Line 1838 in eaa168d

matching: find.byKey(const Key('menu item padding')),

As well, one more key time-picker-dial used for TimePickerDialog.

@justinmc justinmc marked this pull request as ready for review June 24, 2025 17:43
@justinmc justinmc mentioned this pull request Jun 24, 2025
Closed
@justinmc
Copy link
Contributor Author

justinmc commented Jun 24, 2025

@jtmcdole proposed enforcing this via a lint rule or something like that. Issue here: #171099

@github-actions github-actions bot added the a: internationalization Supporting other languages or locales. (aka i18n) label Jun 24, 2025
@justinmc justinmc changed the title No Keys for tests? Avoid using a Key whose only purpose is to be looked up in a test Jun 25, 2025
@dkwingsmt dkwingsmt self-requested a review June 25, 2025 18:34
dkwingsmt
dkwingsmt approved these changes Jun 25, 2025
View reviewed changes
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome change!

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jun 25, 2025 •
edited
Loading

Honestly I think we should forbid any key literals in our widget implementation. Considering how your title is phrased, are there any other key literals left?

@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jun 26, 2025
@justinmc justinmc mentioned this pull request Jun 26, 2025
Closed
@justinmc
Copy link
Contributor Author

justinmc commented Jun 26, 2025

@dkwingsmt Good point, I've opened #171249 to track this. It looks like there are a couple more that we could probably refactor.

Merged via the queue into flutter:master with commit 142cce6 Jun 26, 2025
71 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 26, 2025
@justinmc justinmc deleted the no-keys branch June 26, 2025 23:54
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 27, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 27, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
afdd357 
…test (flutter/flutter#170952)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 27, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 27, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
8213554 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 1, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
5d3ad42 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 1, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
5afe988 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
786697c 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
9cadffa 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
e8fced8 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
dd4e84b 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
c3815c8 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 3, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
456e4ac 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 3, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
6ae4499 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 3, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
08157c0 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 3, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
b44e8f3 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 3, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
b19481a 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 4, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
3ddfe3c 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 4, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
cacd50c 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 4, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
26e6f67 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 5, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
eab062c 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 5, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
14b122e 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
01a4965 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
a9c9474 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 6, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
bdf45a1 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
d3cc3ba 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
004e1fa 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 7, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
0531bef 
…test (flutter/flutter#170952)
mboetger pushed a commit to mboetger/flutter that referenced this pull request Jul 21, 2025
@justinmc @mboetger
Avoid using a Key whose only purpose is to be looked up in a test (fl…
2cea382 
…utter#170952)

This PR investigates moving away from the few places where we rely on
Keys to instead solely look up a widget in a test. Reasons:

 * No extra stuff in users' apps that they don't need for production.
* Avoid collisions between Keys in users' apps that might have the same
identifier string.

I'm definitely still up for discussion on whether or not this is a good
idea!

Written in response to the discussion on
flutter#170761 (comment).
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
c580637 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
87f2120 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
32b932d 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
bd18c88 
…test (flutter/flutter#170952)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
@engine-flutter-autoroll
142cce6 Avoid using a Key whose only purpose is to be looked up in a …
9f6aca3 
…test (flutter/flutter#170952)
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
@justinmc @lucaantonelli
Avoid using a Key whose only purpose is to be looked up in a test (fl…
0597cd0 
…utter#170952)

This PR investigates moving away from the few places where we rely on
Keys to instead solely look up a widget in a test. Reasons:

 * No extra stuff in users' apps that they don't need for production.
* Avoid collisions between Keys in users' apps that might have the same
identifier string.

I'm definitely still up for discussion on whether or not this is a good
idea!

Written in response to the discussion on
flutter#170761 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Assignees

@justinmc justinmc

Labels

a: internationalization Supporting other languages or locales. (aka i18n) f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justinmc @AhmedLSayed9 @dkwingsmt