Skip to content

Conversation

@spkersten
Copy link
Contributor

@spkersten spkersten commented Aug 17, 2021

Relates to dart-lang/sdk#29276

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 feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Aug 17, 2021
@google-cla google-cla bot added the cla: yes label Aug 17, 2021
@spkersten spkersten force-pushed the const-multi-child-render-object-widget branch from 396e46c to de7a34e Compare August 17, 2021 15:54
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have expected that const can on line 32, but that results in:

error: Evaluation of this constant expression throws an exception. (const_eval_throws_exception at [flutter] test/widgets/list_body_test.dart:49)

@spkersten spkersten force-pushed the const-multi-child-render-object-widget branch from de7a34e to e96c309 Compare August 17, 2021 16:06
@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. labels Aug 17, 2021
@spkersten spkersten force-pushed the const-multi-child-render-object-widget branch from e96c309 to bba6858 Compare August 17, 2021 16:19
@goderbauer goderbauer self-requested a review August 19, 2021 18:48
@spkersten
Copy link
Contributor Author

@goderbauer Since the PR has been open for a while, is this waiting for anything from me? (Besides the merge conflict.)

@goderbauer
Copy link
Member

Sorry for the delay.

Is the reasoning here that we can remove the check because in an NNBD world List<Widget> children can never contain null? We've left these kind of null checks in the code base for now because not all apps have migrated to NNBD yet and they still need these asserts for proper error reporting.

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@gustav3d
Copy link

Sorry for the delay.

Is the reasoning here that we can remove the check because in an NNBD world List<Widget> children can never contain null? We've left these kind of null checks in the code base for now because not all apps have migrated to NNBD yet and they still need these asserts for proper error reporting.

For how long will this be the case ?.
That some people dont fix their codebase to be NNBD but still expect to function in the otherwise evolving flutter versions is to prevent flutter itself to be pure NNBD.
Why not simple dictate that after version X, its pure NNBD, people who refusee to adapt are stuck on earlier version.

@albertodev01
Copy link
Contributor

Is there any update on this? Having const available for rows, columns, stacks, containers and much more would be a huge lift

@albertodev01
Copy link
Contributor

Done! See #118837

@gustav3d
Copy link

Thanks ! today is a good day :)

@goderbauer
Copy link
Member

It'll still requires a little more work before it is truly done, but we're on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants