This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Remove scheduleBuild() #239
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I've noticed an anti-pattern emerge where people call scheduleBuild() when they think they've changed enough state that they should rebuild, instead of just wrapping their changes in setState(). This leads to them missing state changes, having extraneous scheduleBuild() calls, and other similar bugs. By removing scheduleBuild(), the only way to actually schedule a build now is to call setState(), and hopefully that'll make it much clearer that you're only supposed to do this when you change state.
Contributor
|
LGTM |
miguelfrde
pushed a commit
to miguelfrde/engine
that referenced
this pull request
Jan 7, 2020
…ter#239) This prepares the buildroot to stamp GN targets for Fuchsia SDK defined targets. GN templates have been created in //build/fuchsia/fuchsia.gni that can read the JSON metadata that describes the vended SDK. GN targets that describe all known SDK parts (as described in the SDK manifest) can be instantiated in one shot by defining a top level fuchsia_sdk target. This stamping needs to happen once and can be done anywhere (currenly in //build/fuchsia/BUILD.gn). Once stamped, targets that depend on specific Fuchsia SDK parts need to explicitly depend on them by using the name of that part. For example, depending on the :fuchsia.images part of the SDK will generate headers for all fidl files in that part, compile any source libraries and link the required dynamic libraries into the executable. Unlike regular targets, this target does not exist in a GN file. The name must be looked up from JSON manifest. This scheme seems to work well but is not resilient to part name collision. So that SDK and non-SDK Fuchsia builds may co-exist till the migration is done, the is_fuchsia_sdk GN variable and FUCHSIA_SDK preprocessor defines are set in all Fuchsia SDK builds. This allows code to target both variants. It is the hope that the non SDK variant will be turned down and the Flutter team assumes ownership of this component.
rhencke
pushed a commit
to rhencke/engine
that referenced
this pull request
Dec 20, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've noticed an anti-pattern emerge where people call scheduleBuild()
when they think they've changed enough state that they should rebuild,
instead of just wrapping their changes in setState(). This leads to
them missing state changes, having extraneous scheduleBuild() calls,
and other similar bugs.
By removing scheduleBuild(), the only way to actually schedule a build
now is to call setState(), and hopefully that'll make it much clearer
that you're only supposed to do this when you change state.