Skip to content

Commit

Permalink
warn consumers if themed styles are misused (p2). fix #1005
Browse files Browse the repository at this point in the history
  • Loading branch information
iamstarkov committed Jan 30, 2019
1 parent 341c9b2 commit 420dc36
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 25 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- [jss] Simplify cloneStyle function ([#1003](https://github.com/cssinjs/jss/pull/1003))
- [internal] Improve publish script for creating github release ([#999](https://github.com/cssinjs/jss/pull/999))
- [react-jss] Warn about themed styles misuse ([#1006](https://github.com/cssinjs/jss/pull/1006))

### Bug fixes

Expand Down
3 changes: 2 additions & 1 deletion packages/react-jss/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"jss": "10.0.0-alpha.9",
"jss-preset-default": "10.0.0-alpha.9",
"prop-types": "^15.6.0",
"theming": "^3.0.3"
"theming": "^3.0.3",
"tiny-warning": "^1.0.2"
},
"devDependencies": {
"@types/react": "^16.7.20"
Expand Down
37 changes: 13 additions & 24 deletions packages/react-jss/src/withStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import React, {Component, type ComponentType, type Node} from 'react'
import hoistNonReactStatics from 'hoist-non-react-statics'
import {getDynamicStyles, SheetsManager, type StyleSheet} from 'jss'
import {ThemeContext} from 'theming'
import warning from 'tiny-warning'

import type {HOCProps, Options, Styles, InnerProps} from './types'
import getDisplayName from './getDisplayName'
Expand Down Expand Up @@ -35,32 +36,20 @@ let managersCounter = 0

const NoRenderer = (props: {children?: Node}) => props.children || null

const getStyles = <Theme: {}>(styles: Styles<Theme>, theme: Theme) => {
const getStyles = <Theme: {}>(styles: Styles<Theme>, theme: Theme, displayName: string) => {
if (typeof styles !== 'function') {
return styles
}
if (process.env.NODE_ENV !== 'production') {
if (styles.length < 1) {
console.warn(
`
Warning: [JSS]: This Component uses themed styles notation (function) while not relying on a theme (0 arguments).
It slows your app down and you should rewrite these styles to plain object notation.
`
.replace(/[\s]{2,}/gim, ' ')
.trim()
)
}
if (styles.length > 1) {
console.warn(
`
Warning: [JSS]: This Component's themed styles expect more than 1 argument.
React-jss provides only one argument.
`
.replace(/[\s]{2,}/gim, ' ')
.trim()
)
}
}
warning(
styles.length < 1,
`Warning: [JSS]: <${displayName} />'s styles function doesn't rely on a theme. We recommend to rewrite it to plain object.\nRead more: https://github.com/cssinjs/jss/blob/master/docs/react-jss.md#basic`
)
warning(
styles.length > 1,
`Warning: [JSS]: <${displayName} />'s styles function uses ${
styles.length
} arguments, but react-jss passes only theme as 1st and only argument. We recommend to rewrite it to take only one argument.\nRead more: https://github.com/cssinjs/jss/blob/master/docs/react-jss.md#theming`
)

return styles(theme)
}
Expand Down Expand Up @@ -157,7 +146,7 @@ export default function withStyles<Theme: {}, S: Styles<Theme>>(
return staticSheet
}

const themedStyles = getStyles(styles, theme)
const themedStyles = getStyles(styles, theme, displayName)
const contextSheetOptions = this.props.jssContext.sheetOptions
staticSheet = this.jss.createStyleSheet(themedStyles, {
...sheetOptions,
Expand Down

0 comments on commit 420dc36

Please sign in to comment.