Optimize script tags (vercel#9048)

* Update _document.tsx to optimize script loading * Update maxInitialChunks to 15 * Undo change chunk count * Adjust impl a bit * Add CSS preload test * Ensure build manifest not preloaded * Correct file name * break comment
dmouse · Oct 16, 2019 · 0886a12 · 0886a12
1 parent d3cbb16
commit 0886a12
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 43 deletions.
diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts
@@ -234,7 +234,7 @@ export default async function getBaseWebpackConfig(
           // TODO(atcastle): Analyze if other cache groups should be set to 'all' as well
           chunks: 'all',
           name: 'framework',
-          // This regex ignores nested copies of framework libraries so they're 
+          // This regex ignores nested copies of framework libraries so they're
           // bundled with their issuer.
           // https://github.com/zeit/next.js/pull/9012
           test: /(?<!node_modules.*)[\\/]node_modules[\\/](react|react-dom|scheduler|prop-types)[\\/]/,

diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx
@@ -137,24 +137,32 @@ export class Head extends Component<
 
   context!: React.ContextType<typeof DocumentComponentContext>
 
-  getCssLinks() {
+  getCssLinks(): JSX.Element[] | null {
     const { assetPrefix, files } = this.context._documentProps
     const cssFiles =
       files && files.length ? files.filter(f => /\.css$/.test(f)) : []
 
-    return cssFiles.length === 0
-      ? null
-      : cssFiles.map((file: string) => {
-          return (
-            <link
-              key={file}
-              nonce={this.props.nonce}
-              rel="stylesheet"
-              href={`${assetPrefix}/_next/${encodeURI(file)}`}
-              crossOrigin={this.props.crossOrigin || process.crossOrigin}
-            />
-          )
-        })
+    const cssLinkElements: JSX.Element[] = []
+    cssFiles.forEach(file => {
+      cssLinkElements.push(
+        <link
+          nonce={this.props.nonce}
+          rel="preload"
+          href={`${assetPrefix}/_next/${encodeURI(file)}`}
+          as="style"
+          crossOrigin={this.props.crossOrigin || process.crossOrigin}
+        />,
+        <link
+          key={file}
+          nonce={this.props.nonce}
+          rel="stylesheet"
+          href={`${assetPrefix}/_next/${encodeURI(file)}`}
+          crossOrigin={this.props.crossOrigin || process.crossOrigin}
+        />
+      )
+    })
+
+    return cssLinkElements.length === 0 ? null : cssLinkElements
   }
 
   getPreloadDynamicChunks() {
@@ -189,37 +197,42 @@ export class Head extends Component<
     )
   }
 
-  getPreloadMainLinks() {
+  getPreloadMainLinks(): JSX.Element[] | null {
     const { assetPrefix, files } = this.context._documentProps
-    if (!files || files.length === 0) {
-      return null
-    }
     const { _devOnlyInvalidateCacheQueryString } = this.context
 
-    return files
-      .map((file: string) => {
-        // `dynamicImports` will contain both `.js` and `.module.js` when the
-        // feature is enabled. This clause will filter down to the modern
-        // variants only.
-        // This also filters out non-JS assets.
-        if (!file.endsWith(getOptionalModernScriptVariant('.js'))) {
-          return null
-        }
+    const preloadFiles =
+      files && files.length
+        ? files.filter((file: string) => {
+            // `dynamicImports` will contain both `.js` and `.module.js` when
+            // the feature is enabled. This clause will filter down to the
+            // modern variants only.
+            //
+            // Also filter out _buildManifest because it should not be
+            // preloaded for performance reasons.
+            return (
+              file.endsWith(getOptionalModernScriptVariant('.js')) &&
+              !file.includes('_buildManifest')
+            )
+          })
+        : []
 
-        return (
-          <link
-            key={file}
-            nonce={this.props.nonce}
-            rel="preload"
-            href={`${assetPrefix}/_next/${encodeURI(
-              file
-            )}${_devOnlyInvalidateCacheQueryString}`}
-            as="script"
-            crossOrigin={this.props.crossOrigin || process.crossOrigin}
-          />
-        )
-      })
-      .filter(Boolean)
+    return preloadFiles.length === 0
+      ? null
+      : preloadFiles.map((file: string) => {
+          return (
+            <link
+              key={file}
+              nonce={this.props.nonce}
+              rel="preload"
+              href={`${assetPrefix}/_next/${encodeURI(
+                file
+              )}${_devOnlyInvalidateCacheQueryString}`}
+              as="script"
+              crossOrigin={this.props.crossOrigin || process.crossOrigin}
+            />
+          )
+        })
   }
 
   render() {

diff --git a/test/integration/chunking/test/index.test.js b/test/integration/chunking/test/index.test.js
@@ -3,6 +3,7 @@
 import { join } from 'path'
 import { nextBuild } from 'next-test-utils'
 import { readdir, readFile, unlink, access } from 'fs-extra'
+import cheerio from 'cheerio'
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1
 
@@ -68,6 +69,20 @@ describe('Chunking', () => {
     ).toBe(undefined) /* fs.access callback returns undefined if file exists */
   })
 
+  it('should not preload the build manifest', async () => {
+    const indexPage = await readFile(
+      join(appDir, '.next', 'server', 'static', buildId, 'pages', 'index.html')
+    )
+
+    const $ = cheerio.load(indexPage)
+    expect(
+      [].slice
+        .call($('link[rel="preload"][as="script"]'))
+        .map(e => e.attribs.href)
+        .some(entry => entry.includes('_buildManifest'))
+    ).toBe(false)
+  })
+
   it('should not include more than one instance of react-dom', async () => {
     const misplacedReactDom = stats.chunks.some(chunk => {
       if (chunk.names.includes('framework')) {

diff --git a/test/integration/css/test/index.test.js b/test/integration/css/test/index.test.js
@@ -11,9 +11,11 @@ import {
   startApp,
   stopApp,
   File,
-  waitFor
+  waitFor,
+  renderViaHTTP
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
+import cheerio from 'cheerio'
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
 
@@ -463,6 +465,19 @@ describe('CSS Support', () => {
         }
       }
     })
+
+    it(`should've preloaded the CSS file and injected it in <head>`, async () => {
+      const content = await renderViaHTTP(appPort, '/page2')
+      const $ = cheerio.load(content)
+
+      const cssPreload = $('link[rel="preload"][as="style"]')
+      expect(cssPreload.length).toBe(1)
+      expect(cssPreload.attr('href')).toMatch(/^\/_next\/static\/css\/.*\.css$/)
+
+      const cssSheet = $('link[rel="stylesheet"]')
+      expect(cssSheet.length).toBe(1)
+      expect(cssSheet.attr('href')).toMatch(/^\/_next\/static\/css\/.*\.css$/)
+    })
   })
 
   describe('CSS URL via `file-loader', () => {