Continuation of Flow updates 0.57+ #9203
Merged
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
|
A sample of the API diff it's introducing: | Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| classes | Object | | Useful to extend the style applied to components. |
-| <span style="color: #31a148">component *</span> | ElementType | 'div' | The component used for the root node. Either a string to use a DOM element or a component. |
+| <span style="color: #31a148">component *</span> | ElementType | 'div': ElementType | The component used for the root node. Either a string to use a DOM element or a component. |I'm wondering if we can avoid |
I think we'll have to hack off the flow type cast when we process it. |
oliviertassinari
added
the
PR: needs revision
4 tasks
rosskevin
changed the title
type DefaultProps
sibelius
approved these changes
Nov 19, 2017
rosskevin
removed
PR: needs revision
…to flow-defaultprops # Conflicts: # pages/api/dialog.md # pages/api/drawer.md # pages/api/fade.md # pages/api/grid.md # pages/api/slide.md # pages/api/snackbar.md
This was referenced Nov 20, 2017
|
With this PR, docs should be back to normal. We are now on a temp branch package I have put in an issue for that. |
| @@ -34,6 +34,9 @@ export type Color = 'inherit' | 'accent' | 'action' | 'contrast' | 'disabled' | | |||
|
|
|||
| type ProvidedProps = { | |||
| classes: Object, | |||
| /** | |||
| * @ignore | |||
| */ | |||
| theme?: Object, | |||
There was a problem hiding this comment.
I have been wondering, why to we put the theme here in the first place? I have noticed we can remove them with no flow issue.
There was a problem hiding this comment.
If we can remove them now with @rsolomon's latest PR to react-flow-types, we should do that.
|
@rosskevin Awesome, thank you! |
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.
Closes #9222
This is a follow-on for #8983 by @rsolomon
Now that flow's treatment of
defaultPropschanged with flow0.57.0, we no longer need to specifytype DefaultPropsseparate fromtype Props.The only quirk is that sometimes when a defaultProp is a union, it needs to be cast to the type specified in
type Props, as it doesn't seem to be able to resolve it. Regardless, overall, it's a cut in code and due to reduced duplication.Also, since
type Propsis the single source of truth, I have fixed the maybe types that were missing in #8983 (due to the difficulty of duplicating these properties in bothPropsandDefaultProps)Regarding
SwitchBase, it was used as a typicalComponentin all butRadio, and even inRadio, it was effectively used as a normal component there. It raised some stateless function flow errors so I simply removed the factory wrapper and exposed the underlyingSwitchBase, it left usage virtually unchanged.