Skip to content
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

Get rid of eval #1611

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
a0945c7
Introduce script tag based page loading system.
arunoda Apr 3, 2017
8938843
Call ensurePage only in the dev mode.
arunoda Apr 3, 2017
c95d2b2
Implement router using the page-loader.
arunoda Apr 4, 2017
e3d68ff
Fix a typo and remove unwanted code.
arunoda Apr 4, 2017
03209d8
Fix some issues related to rendering.
arunoda Apr 4, 2017
a1f11a4
Fix production tests.
arunoda Apr 5, 2017
6f65a05
Fix ondemand test cases.
arunoda Apr 5, 2017
57e3a67
Fix unit tests.
arunoda Apr 5, 2017
822a99b
Get rid of eval completely.
arunoda Apr 5, 2017
76bfc38
Remove all the inline code.
arunoda Apr 5, 2017
5aa02d0
Remove the json-pages plugin.
arunoda Apr 6, 2017
32cdf3f
Rename NEXT_PAGE_LOADER into __NEXT_PAGE_LOADER__
arunoda Apr 6, 2017
65a2603
Rename NEXT_LOADED_PAGES into __NEXT_LOADED_PAGES__
arunoda Apr 6, 2017
f6f175d
Remove some unwanted code.
arunoda Apr 6, 2017
e46edc2
Load everything async.
arunoda Apr 6, 2017
751d2e9
Remove lib/eval-script.js
arunoda Apr 6, 2017
4f26e84
Move webpack idle wait code to the page-loader.
arunoda Apr 6, 2017
866319c
Merge with master.
arunoda Apr 6, 2017
6f9b51f
Remove pageNotFound key from the error.
arunoda Apr 11, 2017
b5bd0da
Remove unused error field 'buildError'
arunoda Apr 11, 2017
c077ebe
Add much better logic to normalize routes.
arunoda Apr 11, 2017
fb2f90b
Get rid of mitt.
arunoda Apr 11, 2017
7997c1f
Introduce a better way to register pages.
arunoda Apr 11, 2017
6e0e7b4
Came back to the mitt() based page-loader.
arunoda Apr 11, 2017
95558ae
Add link rel=preload support.
arunoda Apr 12, 2017
6f735ee
Fix a typo.
arunoda Apr 12, 2017
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
53 changes: 36 additions & 17 deletions client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import mitt from 'mitt'
import HeadManager from './head-manager'
import { createRouter } from '../lib/router'
import App from '../lib/app'
import evalScript from '../lib/eval-script'
import { loadGetInitialProps, getURL } from '../lib/utils'
import ErrorDebugComponent from '../lib/error-debug'
import PageLoader from '../lib/page-loader'

// Polyfill Promise globally
// This is needed because Webpack2's dynamic loading(common chunks) code
Expand All @@ -19,31 +19,47 @@ if (!window.Promise) {

const {
__NEXT_DATA__: {
component,
errorComponent,
props,
err,
pathname,
query
query,
buildId
},
location
} = window

const Component = evalScript(component).default
const ErrorComponent = evalScript(errorComponent).default
let lastAppProps

export const router = createRouter(pathname, query, getURL(), {
Component,
ErrorComponent,
err
})
const pageLoader = window.__NEXT_PAGE_LOADER__ = new PageLoader(buildId)
if (window.__NEXT_LOADED_PAGES__) {
window.__NEXT_LOADED_PAGES__.forEach((fn) => fn())
delete window.__NEXT_LOADED_PAGES__
Copy link

@bringking bringking Apr 6, 2017

Choose a reason for hiding this comment

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

Not sure if this matters anymore, but at some point it was considered "bad" to use delete since it is hard to optimize by V8 and other compilers. Whenever I see delete, usually I think there might be a better way via re-assignment. Obviously not a blocker, just an observation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, delete is not bad. You need to use it wisely. Delete is bad when used inside a constructor. Because that's de-optimization. That's also valid for when we are creating a lot of objects from that class.

We need to use it wisely. And we need to delete when needed. Anyway, it's not needed here and having it also not a problem :)

}

const headManager = new HeadManager()
const appContainer = document.getElementById('__next')
const errorContainer = document.getElementById('__next-error')

export default () => {
let lastAppProps
export let router
export let ErrorComponent
let Component

export default async () => {
ErrorComponent = await pageLoader.loadPage('/_error')

try {
Component = await pageLoader.loadPage(pathname)
} catch (err) {
console.error(`${err.message}\n${err.stack}`)
Component = ErrorComponent
}

router = createRouter(pathname, query, getURL(), {
pageLoader,
Component,
ErrorComponent,
err
})

const emitter = mitt()

router.subscribe(({ Component, props, hash, err }) => {
Expand All @@ -57,7 +73,10 @@ export default () => {
}

export async function render (props) {
if (props.err) {
// There are some errors we should ignore.
// Next.js rendering logic knows how to handle them.
// These are specially 404 errors
if (props.err && !props.err.ignore) {
await renderError(props.err)
return
}
Expand Down Expand Up @@ -103,7 +122,7 @@ async function doRender ({ Component, props, hash, err, emitter }) {
}

if (emitter) {
emitter.emit('before-reactdom-render', { Component })
emitter.emit('before-reactdom-render', { Component, ErrorComponent })
}

Component = Component || lastAppProps.Component
Expand All @@ -118,6 +137,6 @@ async function doRender ({ Component, props, hash, err, emitter }) {
ReactDOM.render(createElement(App, appProps), appContainer)

if (emitter) {
emitter.emit('after-reactdom-render', { Component })
emitter.emit('after-reactdom-render', { Component, ErrorComponent })
}
}
62 changes: 33 additions & 29 deletions client/next-dev.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,40 @@
import evalScript from '../lib/eval-script'
import 'react-hot-loader/patch'
import ReactReconciler from 'react-dom/lib/ReactReconciler'

const { __NEXT_DATA__: { errorComponent } } = window
const ErrorComponent = evalScript(errorComponent).default

require('react-hot-loader/patch')
import initOnDemandEntries from './on-demand-entries-client'
import initWebpackHMR from './webpack-hot-middleware-client'

const next = window.next = require('./')

const emitter = next.default()
next.default()
.then((emitter) => {
initOnDemandEntries()
initWebpackHMR()

let lastScroll

emitter.on('before-reactdom-render', ({ Component, ErrorComponent }) => {
// Remember scroll when ErrorComponent is being rendered to later restore it
if (!lastScroll && Component === ErrorComponent) {
const { pageXOffset, pageYOffset } = window
lastScroll = {
x: pageXOffset,
y: pageYOffset
}
}
})

emitter.on('after-reactdom-render', ({ Component, ErrorComponent }) => {
if (lastScroll && Component !== ErrorComponent) {
// Restore scroll after ErrorComponent was replaced with a page component by HMR
const { x, y } = lastScroll
window.scroll(x, y)
lastScroll = null
}
})
})
.catch((err) => {
console.error(`${err.message}\n${err.stack}`)
})

// This is a patch to catch most of the errors throw inside React components.
const originalMountComponent = ReactReconciler.mountComponent
Expand All @@ -21,25 +47,3 @@ ReactReconciler.mountComponent = function (...args) {
throw err
}
}

let lastScroll

emitter.on('before-reactdom-render', ({ Component }) => {
// Remember scroll when ErrorComponent is being rendered to later restore it
if (!lastScroll && Component === ErrorComponent) {
const { pageXOffset, pageYOffset } = window
lastScroll = {
x: pageXOffset,
y: pageYOffset
}
}
})

emitter.on('after-reactdom-render', ({ Component }) => {
if (lastScroll && Component !== ErrorComponent) {
// Restore scroll after ErrorComponent was replaced with a page component by HMR
const { x, y } = lastScroll
window.scroll(x, y)
lastScroll = null
}
})
3 changes: 3 additions & 0 deletions client/next.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import next from './'

next()
.catch((err) => {
console.error(`${err.message}\n${err.stack}`)
})
46 changes: 24 additions & 22 deletions client/on-demand-entries-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,33 @@
import Router from '../lib/router'
import fetch from 'unfetch'

Router.ready(() => {
Router.router.events.on('routeChangeComplete', ping)
})
export default () => {
Router.ready(() => {
Router.router.events.on('routeChangeComplete', ping)
})

async function ping () {
try {
const url = `/_next/on-demand-entries-ping?page=${Router.pathname}`
const res = await fetch(url)
const payload = await res.json()
if (payload.invalid) {
location.reload()
async function ping () {
try {
const url = `/_next/on-demand-entries-ping?page=${Router.pathname}`
const res = await fetch(url)
const payload = await res.json()
if (payload.invalid) {
location.reload()
}
} catch (err) {
console.error(`Error with on-demand-entries-ping: ${err.message}`)
}
} catch (err) {
console.error(`Error with on-demand-entries-ping: ${err.message}`)
}
}

async function runPinger () {
while (true) {
await new Promise((resolve) => setTimeout(resolve, 5000))
await ping()
async function runPinger () {
while (true) {
await new Promise((resolve) => setTimeout(resolve, 5000))
await ping()
}
}
}

runPinger()
.catch((err) => {
console.error(err)
})
runPinger()
.catch((err) => {
console.error(err)
})
}
74 changes: 38 additions & 36 deletions client/webpack-hot-middleware-client.js
Original file line number Diff line number Diff line change
@@ -1,48 +1,50 @@
import webpackHotMiddlewareClient from 'webpack-hot-middleware/client?overlay=false&reload=true'
import Router from '../lib/router'

const handlers = {
reload (route) {
if (route === '/_error') {
for (const r of Object.keys(Router.components)) {
const { err } = Router.components[r]
if (err) {
// reload all error routes
// which are expected to be errors of '/_error' routes
Router.reload(r)
export default () => {
const handlers = {
reload (route) {
if (route === '/_error') {
for (const r of Object.keys(Router.components)) {
const { err } = Router.components[r]
if (err) {
// reload all error routes
// which are expected to be errors of '/_error' routes
Router.reload(r)
}
}
return
}
return
}

if (route === '/_document') {
window.location.reload()
return
}
if (route === '/_document') {
window.location.reload()
return
}

Router.reload(route)
},
Router.reload(route)
},

change (route) {
if (route === '/_document') {
window.location.reload()
return
}
change (route) {
if (route === '/_document') {
window.location.reload()
return
}

const { err } = Router.components[route] || {}
if (err) {
// reload to recover from runtime errors
Router.reload(route)
const { err } = Router.components[route] || {}
if (err) {
// reload to recover from runtime errors
Router.reload(route)
}
}
}
}

webpackHotMiddlewareClient.subscribe((obj) => {
const fn = handlers[obj.action]
if (fn) {
const data = obj.data || []
fn(...data)
} else {
throw new Error('Unexpected action ' + obj.action)
}
})
webpackHotMiddlewareClient.subscribe((obj) => {
const fn = handlers[obj.action]
if (fn) {
const data = obj.data || []
fn(...data)
} else {
throw new Error('Unexpected action ' + obj.action)
}
})
}
11 changes: 6 additions & 5 deletions lib/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ import HTTPStatus from 'http-status'
import Head from './head'

export default class Error extends React.Component {
static getInitialProps ({ res, jsonPageRes }) {
const statusCode = res ? res.statusCode : (jsonPageRes ? jsonPageRes.status : null)
return { statusCode }
static getInitialProps ({ res, err }) {
const statusCode = res ? res.statusCode : (err ? err.statusCode : null)
const pageNotFound = statusCode === 404 || (err ? err.pageNotFound : false)
return { statusCode, pageNotFound }
}

render () {
const { statusCode } = this.props
const title = statusCode === 404
const { statusCode, pageNotFound } = this.props
const title = pageNotFound
? 'This page could not be found'
: HTTPStatus[statusCode] || 'An unexpected error has occurred'

Expand Down
18 changes: 0 additions & 18 deletions lib/eval-script.js

This file was deleted.

Loading