From 7e7f2c0a6df2cf6a48fa4b34beb1d5befe13fa54 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Wed, 5 Jun 2019 20:15:42 +0200 Subject: [PATCH] Simplify a few parts of the codebase (#7506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Move client-side dev JS to dev folder * Move eventsource polyfill * Move source-map-support * Move error boundary * Deprecate Container in _app * Make initialRender check better * Remove unused code * Only support one subscription as there is only one * Don’t spread object * Shorten property name * Add container in development too * Simplify query update logic --- package.json | 1 - packages/next-server/lib/router/router.ts | 40 ++++---- packages/next/build/index.ts | 2 +- packages/next/build/output/index.ts | 2 +- packages/next/client/{ => dev}/amp-dev.js | 2 +- .../client/{ => dev}/dev-build-watcher.js | 2 +- .../error-overlay}/eventsource.js | 0 .../format-webpack-messages.d.ts | 0 .../error-overlay}/format-webpack-messages.js | 0 .../error-overlay}/hot-dev-client.js | 2 +- .../error-overlay}/source-map-support.ts | 0 .../client/{ => dev}/event-source-polyfill.js | 0 packages/next/client/{ => dev}/noop.js | 0 .../{ => dev}/on-demand-entries-client.js | 0 .../{ => dev}/on-demand-entries-utils.js | 2 +- .../webpack-hot-middleware-client.js | 2 +- packages/next/client/error-boundary.ts | 12 --- packages/next/client/index.js | 95 +++++++++++++++---- packages/next/client/next-dev.js | 8 +- packages/next/pages/_app.tsx | 59 +----------- packages/next/pages/_document.tsx | 3 +- packages/next/server/hot-reloader.js | 5 +- packages/next/taskfile-babel.js | 2 +- packages/next/types/index.d.ts | 7 +- 24 files changed, 123 insertions(+), 123 deletions(-) rename packages/next/client/{ => dev}/amp-dev.js (97%) rename packages/next/client/{ => dev}/dev-build-watcher.js (98%) rename packages/next/client/{dev-error-overlay => dev/error-overlay}/eventsource.js (100%) rename packages/next/client/{dev-error-overlay => dev/error-overlay}/format-webpack-messages.d.ts (100%) rename packages/next/client/{dev-error-overlay => dev/error-overlay}/format-webpack-messages.js (100%) rename packages/next/client/{dev-error-overlay => dev/error-overlay}/hot-dev-client.js (99%) rename packages/next/client/{ => dev/error-overlay}/source-map-support.ts (100%) rename packages/next/client/{ => dev}/event-source-polyfill.js (100%) rename packages/next/client/{ => dev}/noop.js (100%) rename packages/next/client/{ => dev}/on-demand-entries-client.js (100%) rename packages/next/client/{ => dev}/on-demand-entries-utils.js (94%) rename packages/next/client/{ => dev}/webpack-hot-middleware-client.js (93%) delete mode 100644 packages/next/client/error-boundary.ts diff --git a/package.json b/package.json index d5d1df8b50..8a0db6925e 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,6 @@ ], "*.ts": [ "prettier --write", - "standard --fix --parser typescript-eslint-parser --plugin typescript", "git add" ] }, diff --git a/packages/next-server/lib/router/router.ts b/packages/next-server/lib/router/router.ts index 2df1db0a3a..25a600110d 100644 --- a/packages/next-server/lib/router/router.ts +++ b/packages/next-server/lib/router/router.ts @@ -34,10 +34,12 @@ type RouteInfo = { error?: any } -type Subscription = (data: { App?: ComponentType } & RouteInfo) => void +type Subscription = (data: RouteInfo, App?: ComponentType) => void type BeforePopStateCallback = (state: any) => boolean +type ComponentLoadCancel = (() => void) | null + export default class Router implements BaseRouter { route: string pathname: string @@ -47,8 +49,8 @@ export default class Router implements BaseRouter { * Map of all components loaded in `Router` */ components: { [pathname: string]: RouteInfo } - subscriptions: Set - componentLoadCancel: (() => void) | null + sub: Subscription + clc: ComponentLoadCancel pageLoader: any _bps: BeforePopStateCallback | undefined @@ -64,7 +66,9 @@ export default class Router implements BaseRouter { App, Component, err, + subscription, }: { + subscription: Subscription initialProps: any pageLoader: any Component: ComponentType @@ -95,8 +99,8 @@ export default class Router implements BaseRouter { this.pathname = pathname this.query = query this.asPath = as - this.subscriptions = new Set() - this.componentLoadCancel = null + this.sub = subscription + this.clc = null if (typeof window !== 'undefined') { // in order for `e.state` to work on the `onpopstate` event @@ -522,7 +526,7 @@ export default class Router implements BaseRouter { async fetchComponent(route: string): Promise { let cancelled = false - const cancel = (this.componentLoadCancel = () => { + const cancel = (this.clc = () => { cancelled = true }) @@ -536,8 +540,8 @@ export default class Router implements BaseRouter { throw error } - if (cancel === this.componentLoadCancel) { - this.componentLoadCancel = null + if (cancel === this.clc) { + this.clc = null } return Component @@ -551,7 +555,7 @@ export default class Router implements BaseRouter { const cancel = () => { cancelled = true } - this.componentLoadCancel = cancel + this.clc = cancel const { Component: App } = this.components['/_app'] const props = await loadGetInitialProps>(App, { @@ -560,8 +564,8 @@ export default class Router implements BaseRouter { ctx, }) - if (cancel === this.componentLoadCancel) { - this.componentLoadCancel = null + if (cancel === this.clc) { + this.clc = null } if (cancelled) { @@ -574,20 +578,14 @@ export default class Router implements BaseRouter { } abortComponentLoad(as: string): void { - if (this.componentLoadCancel) { + if (this.clc) { Router.events.emit('routeChangeError', new Error('Route Cancelled'), as) - this.componentLoadCancel() - this.componentLoadCancel = null + this.clc() + this.clc = null } } notify(data: RouteInfo): void { - const { Component: App } = this.components['/_app'] - this.subscriptions.forEach(fn => fn({ ...data, App })) - } - - subscribe(fn: Subscription): () => void { - this.subscriptions.add(fn) - return () => this.subscriptions.delete(fn) + this.sub(data, this.components['/_app'].Component) } } diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 6aff4fbea0..c96a478500 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -11,7 +11,7 @@ import nanoid from 'next/dist/compiled/nanoid/index.js' import path from 'path' import fs from 'fs' import { promisify } from 'util' -import formatWebpackMessages from '../client/dev-error-overlay/format-webpack-messages' +import formatWebpackMessages from '../client/dev/error-overlay/format-webpack-messages' import { recursiveDelete } from '../lib/recursive-delete' import { verifyTypeScriptSetup } from '../lib/verifyTypeScriptSetup' import { CompilerResult, runCompiler } from './compiler' diff --git a/packages/next/build/output/index.ts b/packages/next/build/output/index.ts index 5d4680a635..53d33573dd 100644 --- a/packages/next/build/output/index.ts +++ b/packages/next/build/output/index.ts @@ -3,7 +3,7 @@ import textTable from 'next/dist/compiled/text-table' import createStore from 'next/dist/compiled/unistore' import stripAnsi from 'strip-ansi' -import formatWebpackMessages from '../../client/dev-error-overlay/format-webpack-messages' +import formatWebpackMessages from '../../client/dev/error-overlay/format-webpack-messages' import { OutputState, store as consoleStore } from './store' export function startedDevelopmentServer(appUrl: string) { diff --git a/packages/next/client/amp-dev.js b/packages/next/client/dev/amp-dev.js similarity index 97% rename from packages/next/client/amp-dev.js rename to packages/next/client/dev/amp-dev.js index fde6320349..c07d8521f8 100644 --- a/packages/next/client/amp-dev.js +++ b/packages/next/client/dev/amp-dev.js @@ -1,7 +1,7 @@ /* globals __webpack_hash__ */ import fetch from 'unfetch' import EventSourcePolyfill from './event-source-polyfill' -import { getEventSourceWrapper } from './dev-error-overlay/eventsource' +import { getEventSourceWrapper } from './error-overlay/eventsource' import { setupPing } from './on-demand-entries-utils' if (!window.EventSource) { diff --git a/packages/next/client/dev-build-watcher.js b/packages/next/client/dev/dev-build-watcher.js similarity index 98% rename from packages/next/client/dev-build-watcher.js rename to packages/next/client/dev/dev-build-watcher.js index b3f8a5fe04..1052894258 100644 --- a/packages/next/client/dev-build-watcher.js +++ b/packages/next/client/dev/dev-build-watcher.js @@ -1,4 +1,4 @@ -import { getEventSourceWrapper } from './dev-error-overlay/eventsource' +import { getEventSourceWrapper } from './error-overlay/eventsource' export default function initializeBuildWatcher () { const shadowHost = document.createElement('div') diff --git a/packages/next/client/dev-error-overlay/eventsource.js b/packages/next/client/dev/error-overlay/eventsource.js similarity index 100% rename from packages/next/client/dev-error-overlay/eventsource.js rename to packages/next/client/dev/error-overlay/eventsource.js diff --git a/packages/next/client/dev-error-overlay/format-webpack-messages.d.ts b/packages/next/client/dev/error-overlay/format-webpack-messages.d.ts similarity index 100% rename from packages/next/client/dev-error-overlay/format-webpack-messages.d.ts rename to packages/next/client/dev/error-overlay/format-webpack-messages.d.ts diff --git a/packages/next/client/dev-error-overlay/format-webpack-messages.js b/packages/next/client/dev/error-overlay/format-webpack-messages.js similarity index 100% rename from packages/next/client/dev-error-overlay/format-webpack-messages.js rename to packages/next/client/dev/error-overlay/format-webpack-messages.js diff --git a/packages/next/client/dev-error-overlay/hot-dev-client.js b/packages/next/client/dev/error-overlay/hot-dev-client.js similarity index 99% rename from packages/next/client/dev-error-overlay/hot-dev-client.js rename to packages/next/client/dev/error-overlay/hot-dev-client.js index b983104f45..fc40c762e7 100644 --- a/packages/next/client/dev-error-overlay/hot-dev-client.js +++ b/packages/next/client/dev/error-overlay/hot-dev-client.js @@ -30,7 +30,7 @@ import { getEventSourceWrapper } from './eventsource' import formatWebpackMessages from './format-webpack-messages' import * as ErrorOverlay from 'react-error-overlay' import stripAnsi from 'strip-ansi' -import { rewriteStacktrace } from '../source-map-support' +import { rewriteStacktrace } from './source-map-support' import fetch from 'unfetch' // This alternative WebpackDevServer combines the functionality of: diff --git a/packages/next/client/source-map-support.ts b/packages/next/client/dev/error-overlay/source-map-support.ts similarity index 100% rename from packages/next/client/source-map-support.ts rename to packages/next/client/dev/error-overlay/source-map-support.ts diff --git a/packages/next/client/event-source-polyfill.js b/packages/next/client/dev/event-source-polyfill.js similarity index 100% rename from packages/next/client/event-source-polyfill.js rename to packages/next/client/dev/event-source-polyfill.js diff --git a/packages/next/client/noop.js b/packages/next/client/dev/noop.js similarity index 100% rename from packages/next/client/noop.js rename to packages/next/client/dev/noop.js diff --git a/packages/next/client/on-demand-entries-client.js b/packages/next/client/dev/on-demand-entries-client.js similarity index 100% rename from packages/next/client/on-demand-entries-client.js rename to packages/next/client/dev/on-demand-entries-client.js diff --git a/packages/next/client/on-demand-entries-utils.js b/packages/next/client/dev/on-demand-entries-utils.js similarity index 94% rename from packages/next/client/on-demand-entries-utils.js rename to packages/next/client/dev/on-demand-entries-utils.js index 62e989233b..0782b0245a 100644 --- a/packages/next/client/on-demand-entries-utils.js +++ b/packages/next/client/dev/on-demand-entries-utils.js @@ -1,7 +1,7 @@ /* global window, location */ import fetch from 'unfetch' -import { getEventSourceWrapper } from './dev-error-overlay/eventsource' +import { getEventSourceWrapper } from './error-overlay/eventsource' let evtSource export let currentPage diff --git a/packages/next/client/webpack-hot-middleware-client.js b/packages/next/client/dev/webpack-hot-middleware-client.js similarity index 93% rename from packages/next/client/webpack-hot-middleware-client.js rename to packages/next/client/dev/webpack-hot-middleware-client.js index 97c7aec094..70df323a51 100644 --- a/packages/next/client/webpack-hot-middleware-client.js +++ b/packages/next/client/dev/webpack-hot-middleware-client.js @@ -1,4 +1,4 @@ -import connect from './dev-error-overlay/hot-dev-client' +import connect from './error-overlay/hot-dev-client' export default ({ assetPrefix }) => { const options = { diff --git a/packages/next/client/error-boundary.ts b/packages/next/client/error-boundary.ts deleted file mode 100644 index 6ab0772c33..0000000000 --- a/packages/next/client/error-boundary.ts +++ /dev/null @@ -1,12 +0,0 @@ -import React, { ErrorInfo } from 'react' - -export class ErrorBoundary extends React.Component<{ - fn: (err: Error, info: ErrorInfo) => void -}> { - componentDidCatch(err: Error, info: ErrorInfo) { - this.props.fn(err, info) - } - render() { - return this.props.children - } -} diff --git a/packages/next/client/index.js b/packages/next/client/index.js index 4a02b74e3c..c83813f5a9 100644 --- a/packages/next/client/index.js +++ b/packages/next/client/index.js @@ -6,12 +6,12 @@ import mitt from 'next-server/dist/lib/mitt' import { loadGetInitialProps, getURL } from 'next-server/dist/lib/utils' import PageLoader from './page-loader' import * as envConfig from 'next-server/config' -import { ErrorBoundary } from './error-boundary' import Loadable from 'next-server/dist/lib/loadable' import { HeadManagerContext } from 'next-server/dist/lib/head-manager-context' import { DataManagerContext } from 'next-server/dist/lib/data-manager-context' import { RouterContext } from 'next-server/dist/lib/router-context' import { DataManager } from 'next-server/dist/lib/data-manager' +import { parse as parseQs, stringify as stringifyQs } from 'querystring' // Polyfill Promise globally // This is needed because Webpack's dynamic loading(common chunks) code @@ -71,6 +71,55 @@ export let ErrorComponent let Component let App +class Container extends React.Component { + componentDidCatch (err, info) { + this.props.fn(err, info) + } + + componentDidMount () { + this.scrollToHash() + + // If page was exported and has a querystring + // If it's a dynamic route (/$ inside) or has a querystring + if ( + data.nextExport && + (router.pathname.indexOf('/$') !== -1 || window.location.search) + ) { + // update query on mount for exported pages + router.replace( + router.pathname + + '?' + + stringifyQs({ + ...router.query, + ...parseQs(window.location.search.substr(1)) + }), + asPath + ) + } + } + + componentDidUpdate () { + this.scrollToHash() + } + + scrollToHash () { + let { hash } = window.location + hash = hash && hash.substring(1) + if (!hash) return + + const el = document.getElementById(hash) + if (!el) return + + // If we call scrollIntoView() in here without a setTimeout + // it won't scroll properly. + setTimeout(() => el.scrollIntoView(), 0) + } + + render () { + return this.props.children + } +} + export const emitter = mitt() export default async ({ webpackHMR: passedWebpackHMR } = {}) => { @@ -109,11 +158,10 @@ export default async ({ webpackHMR: passedWebpackHMR } = {}) => { pageLoader, App, Component, - err: initialErr - }) - - router.subscribe(({ App, Component, props, err }) => { - render({ App, Component, props, err, emitter }) + err: initialErr, + subscription: ({ Component, props, err }, App) => { + render({ App, Component, props, err, emitter }) + } }) render({ App, Component, props, err: initialErr, emitter }) @@ -163,10 +211,11 @@ export async function renderError (props) { await doRender({ ...props, err, Component: ErrorComponent, props: initProps }) } -let isInitialRender = true +// If hydrate does not exist, eg in preact. +let isInitialRender = typeof ReactDOM.hydrate === 'function' function renderReactElement (reactEl, domEl) { // The check for `.hydrate` is there to support React alternatives like preact - if (isInitialRender && typeof ReactDOM.hydrate === 'function') { + if (isInitialRender) { ReactDOM.hydrate(reactEl, domEl) isInitialRender = false } else { @@ -207,21 +256,29 @@ async function doRender ({ App, Component, props, err }) { // In development runtime errors are caught by react-error-overlay. if (process.env.NODE_ENV === 'development') { renderReactElement( - Loading...}> - - - - - - - - , + + renderError({ App, err: error }).catch(err => + console.error('Error rendering page: ', err) + ) + } + > + Loading...}> + + + + + + + + + , appContainer ) } else { // In production we catch runtime errors using componentDidCatch which will trigger renderError. renderReactElement( - renderError({ App, err: error }).catch(err => console.error('Error rendering page: ', err) @@ -237,7 +294,7 @@ async function doRender ({ App, Component, props, err }) { - , + , appContainer ) } diff --git a/packages/next/client/next-dev.js b/packages/next/client/next-dev.js index 71dce3ba39..7b3128723d 100644 --- a/packages/next/client/next-dev.js +++ b/packages/next/client/next-dev.js @@ -1,8 +1,8 @@ import initNext, * as next from './' -import EventSourcePolyfill from './event-source-polyfill' -import initOnDemandEntries from './on-demand-entries-client' -import initWebpackHMR from './webpack-hot-middleware-client' -import initializeBuildWatcher from './dev-build-watcher' +import EventSourcePolyfill from './dev/event-source-polyfill' +import initOnDemandEntries from './dev/on-demand-entries-client' +import initWebpackHMR from './dev/webpack-hot-middleware-client' +import initializeBuildWatcher from './dev/dev-build-watcher' // Temporary workaround for the issue described here: // https://github.com/zeit/next.js/issues/3775#issuecomment-407438123 diff --git a/packages/next/pages/_app.tsx b/packages/next/pages/_app.tsx index 4f2594f3e0..64bf06a3de 100644 --- a/packages/next/pages/_app.tsx +++ b/packages/next/pages/_app.tsx @@ -7,12 +7,7 @@ import { AppInitialProps, AppPropsType, } from 'next-server/dist/lib/utils' -import { - default as singletonRouter, - Router, - makePublicRouterInstance, -} from '../client/router' -import { parse as parseQs, stringify as stringifyQs } from 'querystring' +import { Router, makePublicRouterInstance } from '../client/router' export { AppInitialProps } @@ -65,55 +60,9 @@ export default class App

extends React.Component< } } -export class Container extends React.Component { - componentDidMount() { - this.scrollToHash() - - // @ts-ignore __NEXT_DATA__ is global - if (__NEXT_DATA__.nextExport) { - const curQuery = '?' + stringifyQs(singletonRouter.query) - const hasDiffQuery = location.search && curQuery !== location.search - const isDynamic = singletonRouter.pathname.indexOf('/$') !== -1 - if (isDynamic || hasDiffQuery) { - const parsedQuery = parseQs( - location.search.startsWith('?') - ? location.search.substr(1) - : location.search - ) - // update query on mount for exported pages - let qsString = stringifyQs({ - ...singletonRouter.query, - ...parsedQuery, - }) - qsString = qsString ? '?' + qsString : qsString - singletonRouter.replace( - singletonRouter.pathname + qsString, - location.pathname + qsString - ) - } - } - } - - componentDidUpdate() { - this.scrollToHash() - } - - private scrollToHash() { - let { hash } = window.location - hash = hash && hash.substring(1) - if (!hash) return - - const el = document.getElementById(hash) - if (!el) return - - // If we call scrollIntoView() in here without a setTimeout - // it won't scroll properly. - setTimeout(() => el.scrollIntoView(), 0) - } - - render() { - return this.props.children - } +// @deprecated noop for now until removal +export function Container(p: any) { + return p.children } const warnUrl = execOnce(() => { diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 669f0faae5..866fa16ce3 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -207,7 +207,8 @@ export class Head extends Component { // show a warning if Head contains (only in development) if (process.env.NODE_ENV !== 'production') { children = React.Children.map(children, (child: any) => { - const isReactHelmet = child && child.props && child.props['data-react-helmet'] + const isReactHelmet = + child && child.props && child.props['data-react-helmet'] if (child && child.type === 'title' && !isReactHelmet) { console.warn( "Warning: <title> should not be used in _document.js's <Head>. https://err.sh/next.js/no-document-title" diff --git a/packages/next/server/hot-reloader.js b/packages/next/server/hot-reloader.js index 8f674a0086..a615e4e47a 100644 --- a/packages/next/server/hot-reloader.js +++ b/packages/next/server/hot-reloader.js @@ -227,7 +227,10 @@ export default class HotReloader { let additionalClientEntrypoints = {} additionalClientEntrypoints[CLIENT_STATIC_FILES_RUNTIME_AMP] = `.${sep}` + - relativePath(this.dir, join(NEXT_PROJECT_ROOT_DIST_CLIENT, 'amp-dev')) + relativePath( + this.dir, + join(NEXT_PROJECT_ROOT_DIST_CLIENT, 'dev', 'amp-dev') + ) return Promise.all([ getBaseWebpackConfig(this.dir, { diff --git a/packages/next/taskfile-babel.js b/packages/next/taskfile-babel.js index b3c6077f78..4f281cbad2 100644 --- a/packages/next/taskfile-babel.js +++ b/packages/next/taskfile-babel.js @@ -31,7 +31,7 @@ module.exports = function (task) { if (file.base === 'next-dev.js') { output.code = output.code.replace( '// REPLACE_NOOP_IMPORT', - `import('./noop');` + `import('./dev/noop');` ) } diff --git a/packages/next/types/index.d.ts b/packages/next/types/index.d.ts index dd4d15306b..3ae1fde9a8 100644 --- a/packages/next/types/index.d.ts +++ b/packages/next/types/index.d.ts @@ -1,6 +1,11 @@ import React from 'react' -import { NextPageContext, NextComponentType, NextApiResponse, NextApiRequest } from 'next-server/dist/lib/utils' +import { + NextPageContext, + NextComponentType, + NextApiResponse, + NextApiRequest, +} from 'next-server/dist/lib/utils' /// <reference types="node" /> /// <reference types="react" /> -- GitLab