Removing webpack build step (#20405)

* removing webpack build step

* keep copywebpackplugin and add fonts to assets/fonts

* update marketing font path

* update font path

* update font

* remove builtAssets

* remove copying front end code with dist

* move Inter fonts to /assets/fonts/inter

* move copy-webpack-plugin back to deps

Co-authored-by: Mike Surowiec <mikesurowiec@users.noreply.github.com>

.gitignore

@@ -7,14 +7,13 @@ npm-debug.log
 coverage/
 .linkinator
 /assets/images/early-access
+/assets/fonts/inter
 /content/early-access
 /data/early-access
-dist
 .next
 .eslintcache
 
 # blc: broken link checker
 blc_output.log
 blc_output_internal.log
-/dist/
 broken_links.md

Dockerfile

@@ -46,7 +46,6 @@ COPY lib ./lib
 # one part of the build relies on this content file to pull all-products
 COPY content/index.md ./content/index.md
 
-COPY webpack.config.js ./webpack.config.js
 COPY next.config.js ./next.config.js
 COPY tsconfig.json ./tsconfig.json
 
@@ -73,7 +72,6 @@ USER node
 COPY --chown=node:node --from=prod_deps /usr/src/docs/node_modules /usr/src/docs/node_modules
 
 # Copy our front-end code
-COPY --chown=node:node --from=builder /usr/src/docs/dist /usr/src/docs/dist
 COPY --chown=node:node --from=builder /usr/src/docs/.next /usr/src/docs/.next
 
 # We should always be running in production mode

lib/built-asset-urls.js

@@ -1,23 +0,0 @@
-import fs from 'fs'
-import path from 'path'
-import crypto from 'crypto'
-
-// Get an MD4 Digest Hex content hash, loosely based on Webpack `[contenthash]`
-function getContentHash(absFilePath) {
-  const buffer = fs.readFileSync(absFilePath)
-  const hash = crypto.createHash('md4')
-  hash.update(buffer)
-  return hash.digest('hex')
-}
-
-function getUrl(relFilePath) {
-  const absFilePath = path.join(process.cwd(), relFilePath)
-  return `/${relFilePath}?hash=${getContentHash(absFilePath)}`
-}
-
-export default {
-  main: {
-    js: getUrl('dist/index.js'),
-    css: getUrl('dist/index.css'),
-  },
-}

middleware/context.js

@@ -6,7 +6,6 @@ import xPathUtils from '../lib/path-utils.js'
 import productNames from '../lib/product-names.js'
 import warmServer from '../lib/warm-server.js'
 import readJsonFile from '../lib/read-json-file.js'
-import builtAssets from '../lib/built-asset-urls.js'
 import searchVersions from '../lib/search/versions.js'
 import nonEnterpriseDefaultVersion from '../lib/non-enterprise-default-version.js'
 const activeProducts = Object.values(productMap).filter(
@@ -58,9 +57,6 @@ export default async function contextualize(req, res, next) {
   req.context.siteTree = siteTree
   req.context.pages = pageMap
 
-  // JS + CSS asset paths
-  req.context.builtAssets = builtAssets
-
   // Object exposing selected variables to client
   req.context.expose = JSON.stringify({
     // Languages and versions for search

middleware/handle-errors.js

@@ -1,6 +1,5 @@
 import FailBot from '../lib/failbot.js'
 import loadSiteData from '../lib/site-data.js'
-import builtAssets from '../lib/built-asset-urls.js'
 import { nextApp } from './next.js'
 
 function shouldLogException(error) {
@@ -42,9 +41,8 @@ export default async function handleError(error, req, res, next) {
     // set req.context.site here so we can pass data/ui.yml text to the 500 layout
     if (!req.context) {
       const site = await loadSiteData()
-      req.context = { site: site[req.language || 'en'].site, builtAssets }
+      req.context = { site: site[req.language || 'en'].site }
     }
-
     // display error on the page in development and staging, but not in production
     if (req.context && process.env.HEROKU_PRODUCTION_APP !== 'true') {
       req.context.error = error

next.config.js

@@ -3,17 +3,30 @@
 
 const fs = require('fs')
 const frontmatter = require('gray-matter')
+const CopyWebpackPlugin = require('copy-webpack-plugin')
 const path = require('path')
 const homepage = path.posix.join(process.cwd(), 'content/index.md')
 const { data } = frontmatter(fs.readFileSync(homepage, 'utf8'))
 const productIds = data.children
 
 module.exports = {
+  webpack: (config, { isServer }) => {
+    if (isServer) {
+      config.plugins.push(
+        new CopyWebpackPlugin({
+          patterns: [
+            {
+              from: path.join(__dirname, 'node_modules/@primer/css/fonts'),
+              to: path.join(__dirname, 'assets/fonts/inter'),
+            },
+          ],
+        })
+      )
+    }
+    return config
+  },
   // speed up production `next build` by ignoring typechecking during that step of build.
   // type-checking still occurs in the Dockerfile build
-  future: {
-    webpack: 5,
-  },
   typescript: {
     ignoreBuildErrors: true,
   },

package.json

@@ -26,6 +26,7 @@
     "connect-datadog": "0.0.9",
     "connect-slashes": "^1.4.0",
     "cookie-parser": "^1.4.5",
+    "copy-webpack-plugin": "^9.0.1",
     "cors": "^2.8.5",
     "csurf": "^1.11.0",
     "dayjs": "^1.10.4",
@@ -118,7 +119,6 @@
     "babel-preset-env": "^1.7.0",
     "chalk": "^4.1.1",
     "commander": "^7.2.0",
-    "copy-webpack-plugin": "^8.1.1",
     "count-array-values": "^1.2.1",
     "cross-env": "^7.0.3",
     "csp-parse": "0.0.2",
@@ -173,8 +173,6 @@
     "ts-loader": "^9.2.3",
     "typescript": "^4.3.2",
     "url-template": "^2.0.8",
-    "webpack": "^5.37.0",
-    "webpack-cli": "^4.7.0",
     "website-scraper": "^4.2.3"
   },
   "engines": {
@@ -196,7 +194,7 @@
     "browser-test": "start-server-and-test browser-test-server 4001 browser-test-tests",
     "browser-test-server": "cross-env NODE_ENV=production WEB_CONCURRENCY=1 PORT=4001 node server.mjs",
     "browser-test-tests": "cross-env BROWSER=1 NODE_OPTIONS=--experimental-vm-modules jest tests/browser/browser.js",
-    "build": "npm run webpack-build && next build",
+    "build": "next build",
     "debug": "cross-env NODE_ENV=development ENABLED_LANGUAGES='en,ja' nodemon --inspect server.mjs",
     "dev": "npm start",
     "heroku-postbuild": "node script/early-access/clone-for-build.js && npm run build",
@@ -220,8 +218,7 @@
     "sync-search-indices": "script/sync-search-indices.js",
     "sync-search-server": "cross-env NODE_ENV=production WEB_CONCURRENCY=1 PORT=4002 node server.mjs",
     "test": "npm run lint && cross-env NODE_OPTIONS=--experimental-vm-modules jest",
-    "test-watch": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --watch --notify --notifyMode=change --coverage",
+    "test-watch": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --watch --notify --notifyMode=change --coverage"
-    "webpack-build": "cross-env NODE_ENV=production npx webpack --mode production"
   },
   "lint-staged": {
     "*.{js,mjs,ts,tsx}": "eslint --cache --fix",

stylesheets/index.scss

@@ -5,7 +5,7 @@
 @import "@primer/css/labels/index.scss";
 @import "@primer/css/avatars/avatar.scss";
 
-$marketing-font-path: "/dist/fonts/";
+$marketing-font-path: "/assets/fonts/inter/";
 @import "@primer/css/marketing/index.scss";
 @import "font-mktg.scss";
 

tests/rendering/server.js

@@ -3,7 +3,6 @@ import enterpriseServerReleases from '../../lib/enterprise-server-releases.js'
 import { get, getDOM, head, post } from '../helpers/supertest.js'
 import { describeViaActionsOnly } from '../helpers/conditional-runs.js'
 import { loadPages } from '../../lib/page-data.js'
-import builtAssets from '../../lib/built-asset-urls.js'
 import CspParse from 'csp-parse'
 import { productMap } from '../../lib/all-products.js'
 import { jest } from '@jest/globals'
@@ -177,7 +176,11 @@ describe('server', () => {
     const $ = await getDOM('/not-a-real-page')
     expect($('h1').text()).toBe('Ooops!')
     expect($.text().includes("It looks like this page doesn't exist.")).toBe(true)
-    expect($.text().includes('We track these errors automatically, but if the problem persists please feel free to contact us.')).toBe(true)
+    expect(
+      $.text().includes(
+        'We track these errors automatically, but if the problem persists please feel free to contact us.'
+      )
+    ).toBe(true)
     expect($.res.statusCode).toBe(404)
   })
 
@@ -200,8 +203,12 @@ describe('server', () => {
   test('renders a 500 page when errors are thrown', async () => {
     const $ = await getDOM('/_500')
     expect($('h1').text()).toBe('Ooops!')
-    expect($.text().includes("It looks like something went wrong.")).toBe(true)
+    expect($.text().includes('It looks like something went wrong.')).toBe(true)
-    expect($.text().includes("We track these errors automatically, but if the problem persists please feel free to contact us.")).toBe(true)
+    expect(
+      $.text().includes(
+        'We track these errors automatically, but if the problem persists please feel free to contact us.'
+      )
+    ).toBe(true)
     expect($.res.statusCode).toBe(500)
   })
 
@@ -498,9 +505,7 @@ describe('server', () => {
       const $ = await getDOM(
         `${latestEnterprisePath}/github/getting-started-with-github/set-up-git`
       )
-      expect(
+      expect($('a[href="/en/desktop/installing-and-configuring-github-desktop"]').length).toBe(1)
-        $('a[href="/en/desktop/installing-and-configuring-github-desktop"]').length
-      ).toBe(1)
     })
 
     test('admin articles that link to non-admin articles have Enterprise user links', async () => {
@@ -526,16 +531,16 @@ describe('server', () => {
         `${latestEnterprisePath}/admin/installation/upgrading-github-enterprise-server`
       )
       expect(
-        $(
+        $('a[href="https://docs.microsoft.com/azure/backup/backup-azure-vms-first-look-arm"]')
-          'a[href="https://docs.microsoft.com/azure/backup/backup-azure-vms-first-look-arm"]'
+          .length
-        ).length
       ).toBe(1)
     })
   })
 
   describe('article versions', () => {
     test('includes links to all versions of each article', async () => {
-      const articlePath = 'github/setting-up-and-managing-your-github-user-account/managing-user-account-settings/about-your-personal-dashboard'
+      const articlePath =
+        'github/setting-up-and-managing-your-github-user-account/managing-user-account-settings/about-your-personal-dashboard'
       const $ = await getDOM(
         `/en/enterprise-server@${enterpriseServerReleases.latest}/${articlePath}`
       )
@@ -545,7 +550,10 @@ describe('server', () => {
         ).length
       ).toBe(2)
       // 2.13 predates this feature, so it should be excluded:
-      expect($(`[data-testid=article-version-picker] a[href="/en/enterprise/2.13/user/${articlePath}"]`).length).toBe(0)
+      expect(
+        $(`[data-testid=article-version-picker] a[href="/en/enterprise/2.13/user/${articlePath}"]`)
+          .length
+      ).toBe(0)
     })
 
     test('is not displayed if article has only one version', async () => {
@@ -852,8 +860,9 @@ describe('GitHub Desktop URLs', () => {
 
 describe('static assets', () => {
   test('fonts', async () => {
-    expect((await get('/dist/fonts/Inter-Medium.woff')).statusCode).toBe(200)
+    expect((await get('/assets/fonts/inter/Inter-Bold.woff')).statusCode).toBe(200)
-    expect((await get('/dist/fonts/Inter-Regular.woff')).statusCode).toBe(200)
+    expect((await get('/assets/fonts/inter/Inter-Medium.woff')).statusCode).toBe(200)
+    expect((await get('/assets/fonts/inter/Inter-Regular.woff')).statusCode).toBe(200)
   })
 })
 
@@ -948,37 +957,6 @@ describe('?json query param for context debugging', () => {
   })
 })
 
-describe('stylesheets', () => {
-  it('compiles and sets the right content-type header', async () => {
-    const stylesheetUrl = builtAssets.main.css
-    const res = await get(stylesheetUrl)
-    expect(res.statusCode).toBe(200)
-    expect(res.headers['content-type']).toBe('text/css; charset=UTF-8')
-  })
-})
-
-describe('client-side JavaScript bundle', () => {
-  let res
-  beforeAll(async () => {
-    const scriptUrl = builtAssets.main.js
-    res = await get(scriptUrl)
-  })
-
-  it('returns a 200 response', async () => {
-    expect(res.statusCode).toBe(200)
-  })
-
-  it('sets the right content-type header', async () => {
-    expect(res.headers['content-type']).toBe('application/javascript; charset=UTF-8')
-  })
-
-  // TODO: configure webpack to create production bundle in the test env
-  // it('is not too big', async () => {
-  //   const tooBig = 10 * 1000
-  //   expect(res.text.length).toBeLessThan(tooBig)
-  // })
-})
-
 describe('static routes', () => {
   it('serves content from the /assets directory', async () => {
     expect((await get('/assets/images/site/be-social.gif')).statusCode).toBe(200)

webpack.config.js

@@ -1,78 +0,0 @@
-const path = require('path')
-const MiniCssExtractPlugin = require('mini-css-extract-plugin')
-const CopyWebpackPlugin = require('copy-webpack-plugin')
-const { EnvironmentPlugin, ProvidePlugin } = require('webpack')
-
-module.exports = {
-  mode: 'development',
-  devtool: process.env.NODE_ENV === 'development' ? 'eval' : 'source-map', // no 'eval' outside of development
-  entry: './javascripts/index.ts',
-  output: {
-    filename: 'index.js',
-    path: path.resolve(__dirname, 'dist'),
-    publicPath: '/dist',
-  },
-  stats: 'errors-only',
-  resolve: {
-    extensions: ['.tsx', '.ts', '.js', '.css', '.scss'],
-  },
-  module: {
-    rules: [
-      {
-        test: /\.tsx?$/,
-        use: 'ts-loader',
-        exclude: /node_modules/,
-      },
-      {
-        test: /\.css$/i,
-        use: ['style-loader', 'css-loader'],
-      },
-      {
-        test: /\.s[ac]ss$/i,
-        use: [
-          MiniCssExtractPlugin.loader,
-          {
-            loader: 'css-loader',
-            options: {
-              sourceMap: true,
-              url: false,
-            },
-          },
-          {
-            // Needed to resolve image url()s within @primer/css
-            loader: 'resolve-url-loader',
-            options: {},
-          },
-          {
-            loader: 'sass-loader',
-            options: {
-              sassOptions: {
-                quietDeps: true,
-                includePaths: ['./stylesheets', './node_modules'],
-                options: {
-                  sourceMap: true,
-                  sourceMapContents: false,
-                },
-              },
-            },
-          },
-        ],
-      },
-    ],
-  },
-  plugins: [
-    new MiniCssExtractPlugin({
-      filename: 'index.css',
-    }),
-    new CopyWebpackPlugin({
-      patterns: [{ from: 'node_modules/@primer/css/fonts', to: 'fonts' }],
-    }),
-    new EnvironmentPlugin({
-      NODE_ENV: 'development', // use 'development' unless process.env.NODE_ENV is defined
-      DEBUG: false,
-    }),
-    new ProvidePlugin({
-      process: 'process/browser',
-    }),
-  ],
-}