CPLAT-3616 Warn consumers about props / state mutation #249
Conversation
This file contains 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
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
aaronlademann-wf
requested changes
Feb 19, 2019
| @@ -444,6 +444,18 @@ abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiStat | |||
| // ---------------------------------------------------------------------- | |||
| } | |||
|
|
|||
| class WarnOnModify<K, V> extends MapView<K, V> { | |||
There was a problem hiding this comment.
This should be private (add an underscore to the beginning of the name).
greglittlefield-wf
previously requested changes
Feb 19, 2019
Co-Authored-By: joebingham-wk <46691367+joebingham-wk@users.noreply.github.com>
| if (isProps) { | ||
| message = | ||
| ''' | ||
| props["$key"] was updated incorrectly. Never mutate this.props directly, as it can cause unexpected behavior; |
There was a problem hiding this comment.
Does this currently print out to the console all indented? If so, we should wrap it in an unindent.
|
+1, will merge pending travis build |
|
+10 @Workiva/release-management-pp |
This was referenced Mar 5, 2019
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ultimate problem:
With the upcoming change to the component_base class, setting the state / props directly will cause breakages. To give consumers a heads up, we should emit warnings in the browser console when they directly mutate props / state values inside a component instance.
How it was fixed:
Adding a class called WarnOnUpdate that watches for setting props / state directly and throws an error in dev mode using an Assert.
Testing suggestions:
Run the test suite. Consider any ways the tests could return false results or miss a use case.
Potential areas of regression:
None that are known as it's only in dev that the error will be thrown.