-
-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[react-jss] Add flow typings #818
Changes from 21 commits
b4b0523
e008910
bd5348a
887652f
3954fc5
a7cca18
b379272
b1fb062
1d69fce
0c07422
386f2e5
839e1ae
fccbbf0
5f1c1e8
d867aa2
539c3ed
bacb6bd
2b86b8d
ecdf147
4c16374
3ad6097
1a803f9
44d7dd5
9da04d3
3d7d984
fcd4345
629d9f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
// @flow | ||
import PropTypes from 'prop-types' | ||
import * as ns from './ns' | ||
import propTypes from './propTypes' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,22 @@ | ||
import React, {Component} from 'react' | ||
// @flow | ||
import React, {Component, type ComponentType} from 'react' | ||
import PropTypes from 'prop-types' | ||
import defaultTheming from 'theming' | ||
import type {StyleSheet} from 'jss' | ||
import jss, {getDynamicStyles, SheetsManager} from './jss' | ||
import compose from './compose' | ||
import getDisplayName from './getDisplayName' | ||
import * as ns from './ns' | ||
import contextTypes from './contextTypes' | ||
import type { | ||
Options, | ||
Theme, | ||
StylesOrThemer, | ||
InnerProps, | ||
OuterProps, | ||
Context, | ||
SubscriptionId | ||
} from './types' | ||
|
||
const env = process.env.NODE_ENV | ||
|
||
|
@@ -32,7 +43,18 @@ const dynamicStylesNs = Math.random() | |
* | ||
*/ | ||
|
||
const getStyles = (stylesOrCreator, theme) => { | ||
type InjectedProps = { | ||
classes?: {}, | ||
theme?: Theme, | ||
sheet?: {} | ||
} | ||
type State = { | ||
theme: Theme, | ||
dynamicSheet?: StyleSheet, | ||
classes: {} | ||
} | ||
|
||
const getStyles = (stylesOrCreator: StylesOrThemer, theme: Theme) => { | ||
if (typeof stylesOrCreator !== 'function') { | ||
return stylesOrCreator | ||
} | ||
|
@@ -57,60 +79,68 @@ let managersCounter = 0 | |
/** | ||
* Wrap a Component into a JSS Container Component. | ||
* | ||
* IProps: Props of the InnerComponent. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. InnerProps would be probably a better name, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. already using InnerProps though, that's why I had to name it IProps 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets find a better convention, maybe InnerPropsType? |
||
* OProps: The Outer props the HOC accepts. | ||
* | ||
* @param {Object|Function} stylesOrCreator | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove those jsdoc comments now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do some cleanup in another PR. Have some small fixes etc. in mind |
||
* @param {Component} InnerComponent | ||
* @param {Object} [options] | ||
* @return {Component} | ||
*/ | ||
export default (stylesOrCreator, InnerComponent, options = {}) => { | ||
export default function createHOC< | ||
IProps: InnerProps, | ||
C: ComponentType<IProps>, | ||
OProps: OuterProps<IProps, C> | ||
>(stylesOrCreator: StylesOrThemer, InnerComponent: C, options: Options): ComponentType<OProps> { | ||
const isThemingEnabled = typeof stylesOrCreator === 'function' | ||
const {theming = defaultTheming, inject, jss: optionsJss, ...sheetOptions} = options | ||
const injectMap = inject ? toMap(inject) : defaultInjectProps | ||
const {themeListener} = theming | ||
const displayName = getDisplayName(InnerComponent) | ||
const defaultClassNamePrefix = env === 'production' ? undefined : `${displayName}-` | ||
const defaultClassNamePrefix = env === 'production' ? '' : `${displayName}-` | ||
const noTheme = {} | ||
const managerId = managersCounter++ | ||
const manager = new SheetsManager() | ||
const defaultProps = {...InnerComponent.defaultProps} | ||
// $FlowFixMe defaultProps are not defined in React$Component | ||
const defaultProps: InjectedProps = {...InnerComponent.defaultProps} | ||
delete defaultProps.classes | ||
|
||
class Jss extends Component { | ||
class Jss extends Component<OProps, State> { | ||
static displayName = `Jss(${displayName})` | ||
static InnerComponent = InnerComponent | ||
static contextTypes = { | ||
...contextTypes, | ||
...(isThemingEnabled && themeListener.contextTypes) | ||
...(isThemingEnabled ? themeListener.contextTypes : {}) | ||
} | ||
static propTypes = { | ||
innerRef: PropTypes.func | ||
} | ||
static defaultProps = defaultProps | ||
|
||
constructor(props, context) { | ||
constructor(props: OProps, context: Context) { | ||
super(props, context) | ||
const theme = isThemingEnabled ? themeListener.initial(context) : noTheme | ||
|
||
this.state = this.createState({theme}, props) | ||
this.state = this.createState({theme, classes: {}}, props) | ||
} | ||
|
||
componentWillMount() { | ||
this.manage(this.state) | ||
} | ||
|
||
componentDidMount() { | ||
if (isThemingEnabled) { | ||
if (themeListener && isThemingEnabled) { | ||
this.unsubscribeId = themeListener.subscribe(this.context, this.setTheme) | ||
} | ||
} | ||
|
||
componentWillReceiveProps(nextProps, nextContext) { | ||
componentWillReceiveProps(nextProps: OProps, nextContext: Context) { | ||
this.context = nextContext | ||
const {dynamicSheet} = this.state | ||
if (dynamicSheet) dynamicSheet.update(nextProps) | ||
} | ||
|
||
componentWillUpdate(nextProps, nextState) { | ||
componentWillUpdate(nextProps: OProps, nextState: State) { | ||
if (isThemingEnabled && this.state.theme !== nextState.theme) { | ||
const newState = this.createState(nextState, nextProps) | ||
this.manage(newState) | ||
|
@@ -119,15 +149,15 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
} | ||
} | ||
|
||
componentDidUpdate(prevProps, prevState) { | ||
componentDidUpdate(prevProps: OProps, prevState: State) { | ||
// We remove previous dynamicSheet only after new one was created to avoid FOUC. | ||
if (prevState.dynamicSheet !== this.state.dynamicSheet && prevState.dynamicSheet) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same diff reason, fixed |
||
this.jss.removeStyleSheet(prevState.dynamicSheet) | ||
} | ||
} | ||
|
||
componentWillUnmount() { | ||
if (this.unsubscribeId) { | ||
if (themeListener && this.unsubscribeId) { | ||
themeListener.unsubscribe(this.context, this.unsubscribeId) | ||
} | ||
|
||
|
@@ -137,9 +167,11 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
} | ||
} | ||
|
||
setTheme = theme => this.setState({theme}) | ||
setTheme = (theme: Theme) => this.setState({theme}) | ||
unsubscribeId: SubscriptionId | ||
context: Context | ||
|
||
createState({theme, dynamicSheet}, {classes: userClasses}) { | ||
createState({theme, dynamicSheet}: State, {classes: userClasses}): State { | ||
const contextSheetOptions = this.context[ns.sheetOptions] | ||
if (contextSheetOptions && contextSheetOptions.disableStylesGeneration) { | ||
return {theme, dynamicSheet, classes: {}} | ||
|
@@ -161,9 +193,11 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
classNamePrefix | ||
}) | ||
this.manager.add(theme, staticSheet) | ||
// $FlowFixMe Cannot add random fields to instance of class StyleSheet | ||
staticSheet[dynamicStylesNs] = getDynamicStyles(styles) | ||
} | ||
|
||
// $FlowFixMe Cannot access random fields on instance of class StyleSheet | ||
const dynamicStyles = staticSheet[dynamicStylesNs] | ||
|
||
if (dynamicStyles) { | ||
|
@@ -176,6 +210,7 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
}) | ||
} | ||
|
||
// $FlowFixMe InnerComponent can be class or stateless, the latter doesn't have a defaultProps property | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it may have defaultProps property same way as class based There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah but flow doesn't have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. weird, we should look for an issue there for explanation or raise one .... really weird There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The explanation is in the comment above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol ... my day is over :) |
||
const defaultClasses = InnerComponent.defaultProps | ||
? InnerComponent.defaultProps.classes | ||
: undefined | ||
|
@@ -194,7 +229,7 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
return {theme, dynamicSheet, classes} | ||
} | ||
|
||
manage({theme, dynamicSheet}) { | ||
manage({theme, dynamicSheet}: State) { | ||
const contextSheetOptions = this.context[ns.sheetOptions] | ||
if (contextSheetOptions && contextSheetOptions.disableStylesGeneration) { | ||
return | ||
|
@@ -231,8 +266,7 @@ export default (stylesOrCreator, InnerComponent, options = {}) => { | |
|
||
render() { | ||
const {theme, dynamicSheet, classes} = this.state | ||
const {innerRef, ...props} = this.props | ||
|
||
const {innerRef, ...props}: OProps = this.props | ||
const sheet = dynamicSheet || this.manager.get(theme) | ||
|
||
if (injectMap.sheet && !props.sheet) props.sheet = sheet | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,5 @@ | ||
export default Component => Component.displayName || Component.name || 'Component' | ||
// @flow | ||
import type {ComponentType} from 'react' | ||
|
||
export default <P>(Component: ComponentType<P>) => | ||
Component.displayName || Component.name || 'Component' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why mapping from lower case to capital?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh, I think we need to rename it to upper case in the core package in the first place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once it starts uppercase you don't need to add Type suffix either