Skip to content

Commit

Permalink
Remove traces of glamor (#1286)
Browse files Browse the repository at this point in the history
* Remove traces of glamor

As talked about with @rauchg. Glamor takes up around 60KB of the bundle (pre-gzip). Since styled-jsx is the way to go now and we support adding glamor by the user we should remove it as dependency cause it is bundled even when not used.

Added rehydration to the example, since we did that in our code.

There is only one thing I'm not sure about and want to discuss:
what should we do with next/css. Right now I added a throw for when it is imported. I'm not sure if we should do that / some other way to notify the user it has been removed. The reasoning behind the throw is that when we would do a console.warn the user would see 'css.default.<X>' not found because we don't have the glamor dependency anymore.

* Update yarn.lock

* Remove test for styles
  • Loading branch information
timneutkens authored and arunoda committed Feb 26, 2017
1 parent 1417f30 commit 408633c
Show file tree
Hide file tree
Showing 10 changed files with 835 additions and 957 deletions.
3 changes: 0 additions & 3 deletions client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 2,6 @@ import { createElement } from 'react'
import ReactDOM from 'react-dom'
import { EventEmitter } from 'events'
import HeadManager from './head-manager'
import { rehydrate } from '../lib/css'
import { createRouter } from '../lib/router'
import App from '../lib/app'
import evalScript from '../lib/eval-script'
Expand All @@ -13,7 12,6 @@ const {
component,
errorComponent,
props,
ids,
err,
pathname,
query
Expand All @@ -35,7 33,6 @@ const container = document.getElementById('__next')

export default (onError) => {
const emitter = new EventEmitter()
if (ids && ids.length) rehydrate(ids)

router.subscribe(({ Component, props, err }) => {
render({ Component, props, err, emitter }, onError)
Expand Down
2 changes: 1 addition & 1 deletion examples/with-glamor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 7,7 @@
"start": "next start"
},
"dependencies": {
"glamor": "^2.20.12",
"glamor": "^2.20.24",
"next": "^2.0.0-beta",
"react": "^15.4.2",
"react-dom": "^15.4.2"
Expand Down
10 changes: 9 additions & 1 deletion examples/with-glamor/pages/_document.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 4,18 @@ import { renderStatic } from 'glamor/server'
export default class MyDocument extends Document {
static async getInitialProps ({ renderPage }) {
const page = renderPage()
let styles = renderStatic(() => page.html)
const styles = renderStatic(() => page.html)
return { ...page, ...styles }
}

constructor (props) {
super(props)
const { __NEXT_DATA__, ids } = props
if (ids) {
__NEXT_DATA__.ids = this.props.ids
}
}

render () {
return (
<html>
Expand Down
9 changes: 8 additions & 1 deletion examples/with-glamor/pages/index.js
Original file line number Diff line number Diff line change
@@ -1,5 1,12 @@
import React from 'react'
import { style } from 'glamor'
import { style, rehydrate } from 'glamor'

// Adds server generated styles to glamor cache.
// Has to run before any `style()` calls
// '__NEXT_DATA__.ids' is set in '_document.js'
if (typeof window !== 'undefined') {
rehydrate(window.__NEXT_DATA__.ids)
}

export default () => <h1 {...styles.title}>My page</h1>

Expand Down
30 changes: 1 addition & 29 deletions lib/css.js
Original file line number Diff line number Diff line change
@@ -1,29 1 @@
import { deprecated } from './utils'
const css = require('glamor')
// Needed because of the mutation below
delete require.cache[require.resolve('glamor')]

for (const [k, v] of Object.entries(css)) {
if (typeof v === 'function') {
css[k] = deprecated(v, 'Warning: \'next/css\' is deprecated. Please refer to the migration guide: https://github.com/zeit/next.js/wiki/Migrating-from-next-css')
}
}

/**
* Expose style as default and the whole object as properties
* so it can be used as follows:
*
* import css, { merge } from 'next/css'
* css({ color: 'red' })
* merge({ color: 'green' })
* css.merge({ color: 'blue' })
*/

css.default = css.style
Object.keys(css).forEach(key => {
if (key !== 'default') {
css.default[key] = css[key]
}
})

module.exports = css
throw new Error(`'next/css' has been removed in Next.js 2.0. Please refer to the migration guide: https://github.com/zeit/next.js/wiki/Migrating-from-next-css`)
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 61,6 @@
"cross-spawn": "5.0.1",
"del": "2.2.2",
"friendly-errors-webpack-plugin": "1.4.0",
"glamor": "2.20.23",
"glob-promise": "3.1.0",
"htmlescape": "1.1.1",
"http-status": "1.0.1",
Expand Down
30 changes: 5 additions & 25 deletions server/document.js
Original file line number Diff line number Diff line change
@@ -1,37 1,18 @@
import React, { Component, PropTypes } from 'react'
import htmlescape from 'htmlescape'
import { renderStatic } from 'glamor/server'
import flush from 'styled-jsx/server'

export default class Document extends Component {
static getInitialProps ({ renderPage }) {
let head
let rendered
let styles
try {
rendered = renderStatic(() => {
const page = renderPage()
head = page.head
return page.html
})
} finally {
styles = flush()
}
const { html, css, ids } = rendered
const nextCSS = { css, ids, styles }
return { html, head, nextCSS }
const {html, head} = renderPage()
const styles = flush()
return { html, head, styles }
}

static childContextTypes = {
_documentProps: PropTypes.any
}

constructor (props) {
super(props)
const { __NEXT_DATA__, nextCSS } = props
if (nextCSS) __NEXT_DATA__.ids = nextCSS.ids
}

getChildContext () {
return { _documentProps: this.props }
}
Expand All @@ -53,11 34,10 @@ export class Head extends Component {
}

render () {
const { head, nextCSS } = this.context._documentProps
const { head, styles } = this.context._documentProps
return <head>
{(head || []).map((h, i) => React.cloneElement(h, { key: i }))}
{nextCSS && nextCSS.css ? <style dangerouslySetInnerHTML={{ __html: nextCSS.css }} /> : null}
{nextCSS && nextCSS.styles ? nextCSS.styles : null}
{styles || null}
{this.props.children}
</head>
}
Expand Down
6 changes: 0 additions & 6 deletions test/integration/basic/pages/css.js

This file was deleted.

8 changes: 0 additions & 8 deletions test/integration/basic/test/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 28,6 @@ export default function ({ app }, suiteName, render) {
expect(html.includes('I can haz meta tags')).toBeTruthy()
})

test('css helper renders styles', async () => {
const $ = await get$('/css')
const redBox = $('#red-box')

expect(redBox.text()).toBe('This is red')
expect(redBox.attr('class')).toMatch(/^css-/)
})

test('renders styled jsx', async () => {
const $ = await get$('/styled-jsx')
const styleId = $('#blue-box').attr('data-jsx')
Expand Down
Loading

0 comments on commit 408633c

Please sign in to comment.