Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
presets: [
'es2015',
'react',
],
plugins: [
Expand Down
3 changes: 3 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@ parser: babel-eslint

env:
jest: true

globals:
ReactClass: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed, where do we use such a global?

Copy link
Copy Markdown
Member Author

@lttb lttb Apr 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint consider flow types and it works with rules like 'no-undef`, but it dosnt about this flow type. so we need to add it as global.

but maybe there is better solution with some plugin etc., I'll take a look

16 changes: 11 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,21 @@ const PrimaryButton = styled(Button, {
Using base Style Sheet we can share classes between styled primitives.

```js
import { Styled } from 'styled-jss'
import injectSheet from 'react-jss'
import { Styled, injectStyled } from 'styled-jss'

// Base styles, like a regular jss object.
const styled = Styled({
root: {
margin: 10
margin: 10,
'& $baseButton': {
fontSize: 16
}
},
baseButton: {
padding: 10
padding: 10,
'& + &': {
marginLeft; 10
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

}
}
})

Expand All @@ -58,11 +63,12 @@ const PrimaryButton = styled(NormalButton, {
// One can use classes AND styled primitives.
const MyComponent = ({classes}) => (
<div className={classes.root}>
<NormalButton>normal button</NormalButton>
<PrimaryButton>primary button</PrimaryButton>
</div>
)

const MyStyledComponent = injectSheet(styled.styles)(MyComponent)
const MyStyledComponent = injectStyled(styled)(MyComponent)
```

### With custom JSS setup:
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"babel-plugin-transform-class-properties": "^6.23.0",
"babel-plugin-transform-es2015-modules-commonjs": "^6.23.0",
"babel-plugin-transform-object-rest-spread": "^6.23.0",
"babel-preset-es2015": "^6.24.1",
"babel-preset-react": "^6.23.0",
"eslint": "^3.13.0",
"eslint-config-airbnb": "^14.1.0",
Expand Down
85 changes: 46 additions & 39 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,37 @@
import {PureComponent, createElement} from 'react'
import {create as createJss, getDynamicStyles} from 'jss'
import preset from 'jss-preset-default'

import filterProps from './utils/filter-props'
import composeClasses from './utils/compose-classes'
import type {
styledType,
StyledElementAttrsType,
StyledElementType,
tagOrStyledElementTypeype,
StyledElementPropsType
} from './types'

const jssDefault = createJss(preset())

type StyledElementAttrsType = { tag: string, styles: Object }
type StyledElementType = Function & StyledElementAttrsType
type tagOrStyledElementTypeype = string | StyledElementType
type StyledElementPropsType = {
classes: Object,
children: ?any,
className: ?string,
}

const createStyled = (jss?: Function = jssDefault) => (baseStyles: Object = {}) => {
let sheet
let dynamicSheet
const createStyled = (jss?: Function = jssDefault) => (baseStyles: Object = {}): styledType => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styledType should be capitelized, no?

Copy link
Copy Markdown
Member

@kof kof Apr 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also lets use a type JssStyles = Object and type JssStyle = Object and put it into types, once we have a fully typed styles object for jss we can easily replace Object with the new one in types.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const sheets = {}
let counter = 0

const mountSheets = () => {
if (!sheets.staticSheet) {
sheets.staticSheet = jss.createStyleSheet(baseStyles, {
link: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need link: true for static sheet?

meta: 'StaticBaseSheet',
}).attach()

sheets.dynamicSheet = jss.createStyleSheet({}, {
link: true,
meta: 'DynamicComponentSheet',
}).attach()
}
}

const styled = (
tagOrStyledElement: tagOrStyledElementTypeype,
ownStyles: Object
Expand Down Expand Up @@ -46,27 +59,17 @@ const createStyled = (jss?: Function = jssDefault) => (baseStyles: Object = {})
}

componentWillMount() {
if (!sheet) {
sheet = jss.createStyleSheet(baseStyles, {
link: true,
meta: 'StaticBaseSheet',
}).attach()

dynamicSheet = jss.createStyleSheet({}, {
link: true,
meta: 'DynamicComponentSheet',
}).attach()
}
mountSheets()

if (!sheet.getRule(staticTag)) {
sheet.addRule(staticTag, elementStyles)
if (!sheets.staticSheet.getRule(staticTag)) {
sheets.staticSheet.addRule(staticTag, elementStyles)
}

if (dynamicStyles && !dynamicSheet.getRule(this.tagScoped)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of tagScoped: tagId

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe dynamicTagName?

dynamicSheet
if (dynamicStyles && !sheets.dynamicSheet.getRule(this.tagScoped)) {
sheets.dynamicSheet
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just realized we don't need sheets object any more.

Copy link
Copy Markdown
Member Author

@lttb lttb Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need, because we need mutated sheets here too https://github.com/cssinjs/styled-jss/pull/6/files#diff-f336995dd331140b2022485b207c9fc1R10

if we just export them, with reassign we will lost links to new sheets

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we reassign them somewhere else than in mountSheets?? Should evtl mountSheets return them?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, you are right, we can just return them.
will fix it

.detach()
.addRule(this.tagScoped, dynamicStyles)
dynamicSheet
sheets.dynamicSheet
.update(this.tagScoped, this.props)
.attach()
.link()
Expand All @@ -75,34 +78,36 @@ const createStyled = (jss?: Function = jssDefault) => (baseStyles: Object = {})

componentWillReceiveProps(nextProps: StyledElementPropsType) {
if (dynamicStyles) {
dynamicSheet.update(this.tagScoped, nextProps)
sheets.dynamicSheet.update(this.tagScoped, nextProps)
}
}

componentWillUnmount() {
dynamicSheet.deleteRule(this.tagScoped)
sheets.dynamicSheet.deleteRule(this.tagScoped)
}

render() {
if (!sheet) return null
if (!sheets.staticSheet) return null

const {children, className, ...attrs} = this.props

const props = filterProps(attrs)
const tagClass = [
sheet.classes[staticTag],
dynamicSheet.classes[this.tagScoped],
className,
]
.filter(Boolean)
.join(' ')
const tagClass = composeClasses(
sheets.staticSheet.classes[staticTag],
sheets.dynamicSheet.classes[this.tagScoped],
className
)

return createElement(tag, {...props, className: tagClass}, children)
}
}
}

return Object.assign(styled, {styles: baseStyles})
return Object.assign(styled, {
sheets,
mountSheets,
styles: baseStyles
})
}

const defaultStyledCreator = createStyled()
Expand All @@ -114,3 +119,5 @@ export {
}

export default defaultStyled

export {default as injectStyled} from './injectStyled'
26 changes: 26 additions & 0 deletions src/injectStyled.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {createElement} from 'react'

import composeClasses from './utils/compose-classes'
import type {styledType} from './types'

const injectStyled = (styled: styledType) => (InnerComponent: ReactClass<any>) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does ReactClass come from?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is native flow type

styled.mountSheets()

const {sheets} = styled
const {staticSheet, dynamicSheet} = sheets

const classNames = new Set([
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid Set? It needs a polyfil.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we can

...Object.keys(staticSheet.classes),
...Object.keys(dynamicSheet.classes)
])

const classes = [...classNames]
.reduce((acc, name) => ({
...acc,
[name]: composeClasses(staticSheet.classes[name], dynamicSheet.classes[name]),
}), {})

return (...props: any) => createElement(InnerComponent, {sheets, classes, ...props})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

props is always an object, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, will fix it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably not inject sheets at all. What use cases are there anyways? Also in react-jss I inject sheet which refs to dynamicSheet if there is one and to static if there is no. Actually I did it because it was already there for the case people still using props.sheet. From my experience there should be no need to use sheet object in react.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, I made it in relation to injectSheet, but it really doesn't look necessary

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If current sheet prop passing in react-jss makes troubles, we might want to remove it from there also.

}

export default injectStyled
4 changes: 2 additions & 2 deletions src/tests/App.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react'
import type {styledType} from '../types'


export default (styled: Function) => {
export default (styled: styledType) => {
const App = styled('div', {
margin: 50,
})
Expand Down
31 changes: 31 additions & 0 deletions src/tests/__snapshots__/index.spec.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,34 @@ exports[`test renders correctly App with default styled 1`] = `
</section>
</div>
`;

exports[`test renders correctly App with injectStyled 1`] = `
<div
className="root-0-17">
<div
className="div-1-0-19">
<header
className="header-2-0-20">
<h1
className="h1-5-0-21">
Title
</h1>
</header>
<section
className="section-3-0-22">
<button
className="button-6-0-23 button-11-0-24">
primitive test
</button>
<button
className="button-6-0-23 button-12-0-25">
dynamic primitive test
</button>
</section>
<section
className="section-4-0-26">
Another section
</section>
</div>
</div>
`;
21 changes: 20 additions & 1 deletion src/tests/index.spec.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import renderer from 'react-test-renderer'
import React from 'react'

import styled, {Styled} from '../'
import styled, {Styled, injectStyled} from '../'

import CreateApp from './App'

Expand All @@ -25,3 +25,22 @@ it('renders correctly App with default Styled', () => {

expect(tree).toMatchSnapshot()
})

it('renders correctly App with injectStyled', () => {
const customStyled = Styled({
root: {
fontSize: 16
},
baseButton: {
color: 'red',
},
})

const App = CreateApp(customStyled)
const StyledApp = injectStyled(customStyled)(({classes}) => (
<div className={classes.root}><App /></div>
))
const tree = renderer.create(<StyledApp />).toJSON()

expect(tree).toMatchSnapshot()
})
17 changes: 17 additions & 0 deletions src/types/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
export type styledType = Function & {
sheets: {
// TODO: use types from jss
staticSheet: Object,
dynamicSheet: Object,
},
mountSheets: Function,
styles: Object
}
export type StyledElementAttrsType = {tag: string, styles: Object}
export type StyledElementType = Function & StyledElementAttrsType
export type tagOrStyledElementTypeype = string | StyledElementType
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be capitalized

export type StyledElementPropsType = {
classes: Object,
children: ?any,
className: ?string,
}
1 change: 1 addition & 0 deletions src/utils/compose-classes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default (...args: any) => args.filter(Boolean).join(' ')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just realized, why do we use sometimes camel case and sometimes dash separated file names? In JSS I stick to camel case.

Loading