这是indexloc提供的服务，不要输入任何密码
Skip to content

Migrate flutter_manifest to null safety #80392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 16, 2021
Merged

Migrate flutter_manifest to null safety #80392

merged 2 commits into from
Apr 16, 2021

Conversation

jmagman
Copy link
Member

@jmagman jmagman commented Apr 13, 2021

Part of #71511

@jmagman jmagman added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. a: null-safety Support for Dart's null safety feature labels Apr 13, 2021
@jmagman jmagman self-assigned this Apr 13, 2021
@google-cla google-cla bot added the cla: yes label Apr 13, 2021
@jmagman
Copy link
Member Author

jmagman commented Apr 13, 2021

This library has a lot of casts, so a close review would be appreciated.

@jmagman jmagman requested a review from jonahwilliams April 13, 2021 21:55
jonahwilliams
jonahwilliams reviewed Apr 13, 2021
View reviewed changes
packages/flutter_tools/lib/src/flutter_manifest.dart Outdated
List<DeferredComponent> get deferredComponents => _deferredComponents ??= computeDeferredComponents();
List<DeferredComponent> _deferredComponents;
List<DeferredComponent> computeDeferredComponents() {
List<DeferredComponent>? get deferredComponents => _deferredComponents ??= computeDeferredComponents();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably fine, but its a little weird to do the _deferredComponents ??= trick on a field that could be nullable. Instead, you could make this late final (which lacks many of the problems of regular late:

late final deferredComponents  = computeDeferredComponents();

Then computeDeferredComponents will only ever run once, even if it is null

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, late final. Good call.

jonahwilliams
jonahwilliams reviewed Apr 13, 2021
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

You might be able to simplify some of the casting or improve the safety by changing all usage of dynamic to Object?.

jonahwilliams
jonahwilliams reviewed Apr 13, 2021
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

👻

@jmagman
Copy link
Member Author

jmagman commented Apr 13, 2021 •
edited
Loading

You might be able to simplify some of the casting or improve the safety by changing all usage of dynamic to Object?.

We're at the mercy of yaml package signatures for this file, we'll get cast errors like #80011 on collections. Let me take another look and see if there's anything I can spot that looks safe.

@jonahwilliams
Copy link
Contributor

That is if you use Object, but Object? should be fine.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@jmagman
Copy link
Member Author

jmagman commented Apr 15, 2021

Please get at least one approved review

Sorry, I forgot this wasn't approved yet. Thanks bot bro!

jonahwilliams
jonahwilliams approved these changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM!

@jmagman jmagman merged commit af1fa21 into flutter:master Apr 16, 2021
@jmagman jmagman deleted the flutter-manifest-null branch April 16, 2021 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Assignees

@jmagman jmagman

Labels
a: null-safety Support for Dart's null safety feature c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jmagman @jonahwilliams @fluttergithubbot