UIP-2529 Add validateProps method to UiComponent #88
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
RavenNumber of Findings: 0 |
Codecov Report
@@ Coverage Diff @@
## master #88 +/- ##
==========================================
+ Coverage 94.86% 94.87% +0.01%
==========================================
Files 31 31
Lines 1536 1537 +1
==========================================
+ Hits 1457 1458 +1
Misses 79 79 |
|
+1 |
| /// | ||
| /// Use this as an opportunity validate props during the correct lifecycle of the component. | ||
| @mustCallSuper | ||
| void validateProps([Map appliedProps]) { |
There was a problem hiding this comment.
I think this should be a required positional parameter, so that consumers don't have to null-coalesce the argument in overrides.
| @@ -124,6 +124,16 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component { | |||
| ); | |||
| } | |||
|
|
|||
| /// Validation method called during [componentWillReceiveProps], and [componentWillMount]. | |||
| /// | |||
| /// Use this as an opportunity validate props during the correct lifecycle of the component. | |||
There was a problem hiding this comment.
#nit I think this comment could be a little more descriptive.
/// Throws a [PropError] if [appliedProps] are invalid.
///
/// This is called automatically with during [componentWillReceiveProps] and [componentWillMount],
/// and can also be called manually for custom validation.
///
/// Override with a custom implementation to easily add validation (and don't forget to call super!)
///
/// @mustCallSuper
/// void validateProps(Map appliedProps) {
/// super.validateProps(appliedProps);
///
/// var tProps = typedPropsFactory(appliedProps);
/// if (tProps.items.length.isOdd) {
/// throw new PropError.value(tProps.items, 'items', 'must have an even number of items, because reasons');
/// }
/// }| component.componentWillReceiveProps({}); | ||
|
|
||
| expect(calls, ['onValidateProps']); | ||
| }); |
There was a problem hiding this comment.
These should also test that the correct props maps are passed into them
| @@ -124,6 +124,27 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component { | |||
| ); | |||
| } | |||
|
|
|||
| /// Throws a [PropError] if [appliedProps] are invalid. | |||
| /// | |||
| /// This is called automatically with during [componentWillReceiveProps] and [componentWillMount], | |||
There was a problem hiding this comment.
This is called automatically with during
Sorry, my typo here.
What about:
This is called automatically with the latest props available during
|
Still +1 |
|
+1 |
|
@jacehensley-wf This pull request has merge conflicts, please resolve. |
…s_method/dev # Conflicts: # test/over_react/component_declaration/component_base_test.dart
rmconsole-wf
added
Merge Requirements Unmet
and removed
Merge Conflicts
Merge Requirements Met
labels
|
+1 |
aaronlademann-wf
changed the title
|
QA +10
Merging. |
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:
componentWillReceivePropsandcomponentWillMount.How it was fixed:
validatePropsmethod that is called automatically during those lifecycle methods.Testing suggestions:
Potential areas of regression: