Skip to content
New issue

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

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

Already on GitHub? # to your account

fix!: Make all ComponentSet modifications internal #1877

Merged
merged 5 commits into from
Sep 1, 2022

Conversation

spydon
Copy link
Member

@spydon spydon commented Aug 29, 2022

Description

These methods should never have been exposed, since you bypass the component lifecycle with them.

Checklist

  • The title of my PR starts with a Conventional Commit prefix (fix:, feat:, docs: etc).
  • I have read the Contributor Guide and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples.

Breaking Change

  • Yes, this is a breaking change.

Migration instructions

For most methods Component has the corresponding methods directly on it already.
For example, instead of using component.children.addAll you should do component.addAll.

Related Issues

Depends on #1878

@spydon spydon requested review from st-pasha and a team August 29, 2022 16:11
@spydon spydon mentioned this pull request Aug 29, 2022
6 tasks
Copy link
Contributor

@st-pasha st-pasha left a comment

Choose a reason for hiding this comment

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

I think this is good for now, but in the future it would be safer to define ComponentSet interface explicitly, without inheriting from Iterable -- this way we can control the exposed API more tightly. Plus, it could make creating alternative ComponentSet implementations easier.

spydon added a commit that referenced this pull request Aug 30, 2022
Since removeWhere will be removed from the ComponentSet in #1877 we should expose it on the component.
@spydon spydon enabled auto-merge (squash) September 1, 2022 18:44
@spydon spydon merged commit f26a066 into main Sep 1, 2022
@spydon spydon deleted the spydon/component-modifications-internal branch September 1, 2022 18:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants