From 3abe4ba5e1e833067e3681d6dd7ed2e62511d8f1 Mon Sep 17 00:00:00 2001 From: tarunama Date: Wed, 6 Jan 2021 13:03:50 +0900 Subject: [PATCH 1/5] refactor: add return types, and arrange import syntax. --- packages/next/client/index.tsx | 105 ++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index de8b9b5b7772362..ab88eb647453d3b 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -3,7 +3,7 @@ import '@next/polyfill-module' import React from 'react' import ReactDOM from 'react-dom' import { HeadManagerContext } from '../next-server/lib/head-manager-context' -import mitt from '../next-server/lib/mitt' +import mitt, { MittEmitter } from '../next-server/lib/mitt' import { RouterContext } from '../next-server/lib/router-context' import Router from '../next-server/lib/router/router' import { @@ -61,12 +61,13 @@ const { runtimeConfig, dynamicIds, isFallback, + locale, locales, } = data -let { locale, defaultLocale } = data +let { defaultLocale } = data -const prefix = assetPrefix || '' +const prefix: string = assetPrefix || '' // With dynamic assetPrefix it's no longer possible to set assetPrefix at the build time // So, this is how we do it in the client side at runtime @@ -77,7 +78,7 @@ envConfig.setConfig({ publicRuntimeConfig: runtimeConfig || {}, }) -let asPath = getURL() +let asPath: string = getURL() // make sure not to attempt stripping basePath for 404s if (hasBasePath(asPath)) { @@ -131,7 +132,7 @@ if (process.env.__NEXT_I18N_SUPPORT) { type RegisterFn = (input: [string, () => void]) => void -const pageLoader = new PageLoader(buildId, prefix) +const pageLoader: PageLoader = new PageLoader(buildId, prefix) const register: RegisterFn = ([r, f]) => pageLoader.routeLoader.onEntrypoint(r, f) if (window.__NEXT_P) { @@ -142,14 +143,15 @@ if (window.__NEXT_P) { window.__NEXT_P = [] ;(window.__NEXT_P as any).push = register -const headManager = initHeadManager() -const appElement = document.getElementById('__next') +const headManager: { + mountedInstances: Set + updateHead: (head: JSX.Element[]) => void +} = initHeadManager() +const appElement: HTMLElement | null = document.getElementById('__next') -let lastAppProps: AppProps let lastRenderReject: (() => void) | null let webpackHMR: any export let router: Router -let CachedComponent: React.ComponentType let CachedApp: AppComponent, onPerfEntry: (metric: any) => void class Container extends React.Component<{ @@ -209,7 +211,7 @@ class Container extends React.Component<{ hash = hash && hash.substring(1) if (!hash) return - const el = document.getElementById(hash) + const el: HTMLElement | null = document.getElementById(hash) if (!el) return // If we call scrollIntoView() in here without a setTimeout @@ -227,7 +229,8 @@ class Container extends React.Component<{ } } -export const emitter = mitt() +export const emitter: MittEmitter = mitt() +let CachedComponent: React.ComponentType export default async (opts: { webpackHMR?: any } = {}) => { // This makes sure this specific lines are removed in production @@ -252,12 +255,12 @@ export default async (opts: { webpackHMR?: any } = {}) => { duration, entryType, entries, - }) => { + }): void => { // Combines timestamp with random number for unique ID - const uniqueID = `${Date.now()}-${ + const uniqueID: string = `${Date.now()}-${ Math.floor(Math.random() * (9e12 - 1)) + 1e12 }` - let perfStartEntry + let perfStartEntry: string | undefined if (entries && entries.length) { perfStartEntry = entries[0].startTime @@ -267,7 +270,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { id: id || uniqueID, name, startTime: startTime || perfStartEntry, - value: value == null ? duration : value, + value: value === null ? duration : value, label: entryType === 'mark' || entryType === 'measure' ? 'custom' @@ -384,7 +387,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { } } -export async function render(renderingProps: RenderRouteInfo) { +export async function render(renderingProps: RenderRouteInfo): Promise { if (renderingProps.err) { await renderError(renderingProps) return @@ -411,7 +414,7 @@ export async function render(renderingProps: RenderRouteInfo) { // This method handles all runtime and debug errors. // 404 and 500 errors are special kind of errors // and they are still handle via the main render method. -export function renderError(renderErrorProps: RenderErrorProps) { +export function renderError(renderErrorProps: RenderErrorProps): Promise { const { App, err } = renderErrorProps // In development runtime errors are caught by our overlay @@ -477,8 +480,8 @@ export function renderError(renderErrorProps: RenderErrorProps) { } let reactRoot: any = null -let shouldUseHydrate = typeof ReactDOM.hydrate === 'function' -function renderReactElement(reactEl: JSX.Element, domEl: HTMLElement) { +let shouldUseHydrate: boolean = typeof ReactDOM.hydrate === 'function' +function renderReactElement(reactEl: JSX.Element, domEl: HTMLElement): void { if (process.env.__NEXT_REACT_MODE !== 'legacy') { if (!reactRoot) { const opts = { hydrate: true } @@ -504,7 +507,7 @@ function renderReactElement(reactEl: JSX.Element, domEl: HTMLElement) { } } -function markHydrateComplete() { +function markHydrateComplete(): void { if (!ST) return performance.mark('afterHydrate') // mark end of hydration @@ -522,15 +525,16 @@ function markHydrateComplete() { clearMarks() } -function markRenderComplete() { +function markRenderComplete(): void { if (!ST) return performance.mark('afterRender') // mark end of render - const navStartEntries = performance.getEntriesByName('routeChange', 'mark') + const navStartEntries: PerformanceEntryList = performance.getEntriesByName( + 'routeChange', + 'mark' + ) - if (!navStartEntries.length) { - return - } + if (!navStartEntries.length) return performance.measure( 'Next.js-route-change-to-render', @@ -550,7 +554,7 @@ function markRenderComplete() { ) } -function clearMarks() { +function clearMarks(): void { ;[ 'beforeRender', 'afterHydrate', @@ -566,7 +570,7 @@ function AppContainer({ renderError({ App: CachedApp, err: error }).catch((err) => - console.error('Error rendering page: ', err) + console.error(`Error rendering page: ${err}`) ) } > @@ -581,7 +585,7 @@ function AppContainer({ const wrapApp = (App: AppComponent) => ( wrappedAppProps: Record -) => { +): JSX.Element => { const appProps: AppProps = { ...wrappedAppProps, Component: CachedComponent, @@ -595,8 +599,9 @@ const wrapApp = (App: AppComponent) => ( ) } +let lastAppProps: AppProps function doRender(input: RenderRouteInfo): Promise { - let { App, Component, props, err } = input + let { App, Component, props, err }: RenderRouteInfo = input let styleSheets: StyleSheetTuple[] | undefined = 'initial' in input ? undefined : input.styleSheets Component = Component || lastAppProps.Component @@ -611,7 +616,7 @@ function doRender(input: RenderRouteInfo): Promise { // lastAppProps has to be set before ReactDom.render to account for ReactDom throwing an error. lastAppProps = appProps - let canceled = false + let canceled: boolean = false let resolvePromise: () => void const renderPromise = new Promise((resolve, reject) => { if (lastRenderReject) { @@ -643,17 +648,21 @@ function doRender(input: RenderRouteInfo): Promise { return false } - const currentStyleTags = looseToArray( + const currentStyleTags: HTMLStyleElement[] = looseToArray( document.querySelectorAll('style[data-n-href]') ) - const currentHrefs = new Set( + const currentHrefs: Set = new Set( currentStyleTags.map((tag) => tag.getAttribute('data-n-href')) ) - const noscript = document.querySelector('noscript[data-n-css]') - const nonce = noscript?.getAttribute('data-n-css') + const noscript: Element | null = document.querySelector( + 'noscript[data-n-css]' + ) + const nonce: string | null | undefined = noscript?.getAttribute( + 'data-n-css' + ) - styleSheets.forEach(({ href, text }) => { + styleSheets.forEach(({ href, text }: { href: string; text: any }) => { if (!currentHrefs.has(href)) { const styleTag = document.createElement('style') styleTag.setAttribute('data-n-href', href) @@ -670,7 +679,7 @@ function doRender(input: RenderRouteInfo): Promise { return true } - function onHeadCommit() { + function onHeadCommit(): void { if ( // We use `style-loader` in development, so we don't need to do anything // unless we're in production: @@ -681,11 +690,11 @@ function doRender(input: RenderRouteInfo): Promise { // Ensure this render was not canceled !canceled ) { - const desiredHrefs = new Set(styleSheets.map((s) => s.href)) - const currentStyleTags = looseToArray( - document.querySelectorAll('style[data-n-href]') - ) - const currentHrefs = currentStyleTags.map( + const desiredHrefs: Set = new Set(styleSheets.map((s) => s.href)) + const currentStyleTags: HTMLStyleElement[] = looseToArray< + HTMLStyleElement + >(document.querySelectorAll('style[data-n-href]')) + const currentHrefs: string[] = currentStyleTags.map( (tag) => tag.getAttribute('data-n-href')! ) @@ -699,13 +708,15 @@ function doRender(input: RenderRouteInfo): Promise { } // Reorder styles into intended order: - let referenceNode = document.querySelector('noscript[data-n-css]') + let referenceNode: Element | null = document.querySelector( + 'noscript[data-n-css]' + ) if ( // This should be an invariant: referenceNode ) { - styleSheets.forEach(({ href }) => { - const targetTag = document.querySelector( + styleSheets.forEach(({ href }: { href: string }) => { + const targetTag: Element | null = document.querySelector( `style[data-n-href="${href}"]` ) if ( @@ -734,11 +745,11 @@ function doRender(input: RenderRouteInfo): Promise { } } - function onRootCommit() { + function onRootCommit(): void { resolvePromise() } - const elem = ( + const elem: JSX.Element = ( @@ -791,7 +802,7 @@ function Root({ // Dummy component that we render as a child of Root so that we can // toggle the correct styles before the page is rendered. -function Head({ callback }: { callback: () => void }) { +function Head({ callback }: { callback: () => void }): null { // We use `useLayoutEffect` to guarantee the callback is executed // as soon as React flushes the update. React.useLayoutEffect(() => callback(), [callback]) From 7a33435b050de6b634a1a205e1dac7fdabb339d1 Mon Sep 17 00:00:00 2001 From: tarunama Date: Wed, 6 Jan 2021 13:15:54 +0900 Subject: [PATCH 2/5] perf: eval NODE_ENV once. --- packages/next/client/index.tsx | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index ab88eb647453d3b..90167c34034dec5 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -143,6 +143,8 @@ if (window.__NEXT_P) { window.__NEXT_P = [] ;(window.__NEXT_P as any).push = register +const isProdEnv = process.env.NODE_ENV === 'production' +const isDevEnv = process.env.NODE_ENV === 'development' const headManager: { mountedInstances: Set updateHead: (head: JSX.Element[]) => void @@ -220,7 +222,7 @@ class Container extends React.Component<{ } render() { - if (process.env.NODE_ENV === 'production') { + if (isProdEnv) { return this.props.children } else { const { ReactDevOverlay } = require('@next/react-dev-overlay/lib/client') @@ -234,7 +236,7 @@ let CachedComponent: React.ComponentType export default async (opts: { webpackHMR?: any } = {}) => { // This makes sure this specific lines are removed in production - if (process.env.NODE_ENV === 'development') { + if (isDevEnv) { webpackHMR = opts.webpackHMR } @@ -285,7 +287,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { const pageEntrypoint = // The dev server fails to serve script assets when there's a hydration // error, so we need to skip waiting for the entrypoint. - process.env.NODE_ENV === 'development' && hydrateErr + isDevEnv && hydrateErr ? { error: hydrateErr } : await pageLoader.routeLoader.whenEntrypoint(page) if ('error' in pageEntrypoint) { @@ -293,7 +295,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { } CachedComponent = pageEntrypoint.component - if (process.env.NODE_ENV !== 'production') { + if (!isProdEnv) { const { isValidElementType } = require('react-is') if (!isValidElementType(CachedComponent)) { throw new Error( @@ -306,7 +308,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { initialErr = error } - if (process.env.NODE_ENV === 'development') { + if (isDevEnv) { const { getNodeError } = require('@next/react-dev-overlay/lib/client') // Server-side runtime errors need to be re-thrown on the client-side so // that the overlay is rendered. @@ -379,7 +381,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { err: initialErr, } - if (process.env.NODE_ENV === 'production') { + if (isProdEnv) { render(renderCtx) return emitter } else { @@ -401,7 +403,7 @@ export async function render(renderingProps: RenderRouteInfo): Promise { throw renderErr } - if (process.env.NODE_ENV === 'development') { + if (isDevEnv) { // Ensure this error is displayed in the overlay in development setTimeout(() => { throw renderErr @@ -419,7 +421,7 @@ export function renderError(renderErrorProps: RenderErrorProps): Promise { // In development runtime errors are caught by our overlay // In production we catch runtime errors using componentDidCatch which will trigger renderError - if (process.env.NODE_ENV !== 'production') { + if (!isProdEnv) { // A Next.js rendering runtime error is always unrecoverable // FIXME: let's make this recoverable (error in GIP client-transition) webpackHMR.onUnrecoverableError() @@ -643,7 +645,7 @@ function doRender(input: RenderRouteInfo): Promise { !styleSheets || // We use `style-loader` in development, so we don't need to do anything // unless we're in production: - process.env.NODE_ENV !== 'production' + !isProdEnv ) { return false } @@ -683,7 +685,7 @@ function doRender(input: RenderRouteInfo): Promise { if ( // We use `style-loader` in development, so we don't need to do anything // unless we're in production: - process.env.NODE_ENV === 'production' && + isProdEnv && // We can skip this during hydration. Running it wont cause any harm, but // we may as well save the CPU cycles: styleSheets && From 005ae49e9ce088417eb0e2ca57d939fa9580abd2 Mon Sep 17 00:00:00 2001 From: tarunama Date: Wed, 6 Jan 2021 14:44:56 +0900 Subject: [PATCH 3/5] fix: does not use strict comparison --- packages/next/client/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 90167c34034dec5..511c5379d95f3ad 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -272,7 +272,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { id: id || uniqueID, name, startTime: startTime || perfStartEntry, - value: value === null ? duration : value, + value: value == null ? duration : value, label: entryType === 'mark' || entryType === 'measure' ? 'custom' From 4e672b7d2a11e6ae64f1b149972be1acd79873ac Mon Sep 17 00:00:00 2001 From: tarunama Date: Wed, 6 Jan 2021 15:36:53 +0900 Subject: [PATCH 4/5] fix: does not be assigned process.env.NODE_ENV to variable --- packages/next/client/index.tsx | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index ce96c5bc68f35c8..44441e55775ad00 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -151,8 +151,6 @@ if (window.__NEXT_P) { window.__NEXT_P = [] ;(window.__NEXT_P as any).push = register -const isProdEnv = process.env.NODE_ENV === 'production' -const isDevEnv = process.env.NODE_ENV === 'development' const headManager: { mountedInstances: Set updateHead: (head: JSX.Element[]) => void @@ -230,7 +228,7 @@ class Container extends React.Component<{ } render() { - if (isProdEnv) { + if (process.env.NODE_ENV === 'production') { return this.props.children } else { const { ReactDevOverlay } = require('@next/react-dev-overlay/lib/client') @@ -244,7 +242,7 @@ let CachedComponent: React.ComponentType export default async (opts: { webpackHMR?: any } = {}) => { // This makes sure this specific lines are removed in production - if (isDevEnv) { + if (process.env.NODE_ENV === 'development') { webpackHMR = opts.webpackHMR } @@ -295,7 +293,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { const pageEntrypoint = // The dev server fails to serve script assets when there's a hydration // error, so we need to skip waiting for the entrypoint. - isDevEnv && hydrateErr + process.env.NODE_ENV === 'development' && hydrateErr ? { error: hydrateErr } : await pageLoader.routeLoader.whenEntrypoint(page) if ('error' in pageEntrypoint) { @@ -303,7 +301,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { } CachedComponent = pageEntrypoint.component - if (!isProdEnv) { + if (process.env.NODE_ENV !== 'production') { const { isValidElementType } = require('react-is') if (!isValidElementType(CachedComponent)) { throw new Error( @@ -316,7 +314,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { initialErr = error } - if (isDevEnv) { + if (process.env.NODE_ENV === 'development') { const { getNodeError } = require('@next/react-dev-overlay/lib/client') // Server-side runtime errors need to be re-thrown on the client-side so // that the overlay is rendered. @@ -400,7 +398,7 @@ export default async (opts: { webpackHMR?: any } = {}) => { err: initialErr, } - if (isProdEnv) { + if (process.env.NODE_ENV === 'production') { render(renderCtx) return emitter } else { @@ -422,7 +420,7 @@ export async function render(renderingProps: RenderRouteInfo): Promise { throw renderErr } - if (isDevEnv) { + if (process.env.NODE_ENV === 'development') { // Ensure this error is displayed in the overlay in development setTimeout(() => { throw renderErr @@ -440,7 +438,7 @@ export function renderError(renderErrorProps: RenderErrorProps): Promise { // In development runtime errors are caught by our overlay // In production we catch runtime errors using componentDidCatch which will trigger renderError - if (!isProdEnv) { + if (process.env.NODE_ENV !== 'production') { // A Next.js rendering runtime error is always unrecoverable // FIXME: let's make this recoverable (error in GIP client-transition) webpackHMR.onUnrecoverableError() @@ -664,7 +662,7 @@ function doRender(input: RenderRouteInfo): Promise { !styleSheets || // We use `style-loader` in development, so we don't need to do anything // unless we're in production: - !isProdEnv + process.env.NODE_ENV !== 'production' ) { return false } @@ -704,7 +702,7 @@ function doRender(input: RenderRouteInfo): Promise { if ( // We use `style-loader` in development, so we don't need to do anything // unless we're in production: - isProdEnv && + process.env.NODE_ENV === 'production' && // We can skip this during hydration. Running it wont cause any harm, but // we may as well save the CPU cycles: styleSheets && From c146a8c29b7c426d89bbd2eb802245c9bf03e9d1 Mon Sep 17 00:00:00 2001 From: tarunama Date: Thu, 7 Jan 2021 23:36:21 +0900 Subject: [PATCH 5/5] chore: not use template literals --- packages/next/client/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 44441e55775ad00..cde8d7156644009 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -589,7 +589,7 @@ function AppContainer({ renderError({ App: CachedApp, err: error }).catch((err) => - console.error(`Error rendering page: ${err}`) + console.error('Error rendering page: ', err) ) } >