From 6f6ddaa0463dc59812a92ff2b5552bcaadb98bdd Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 2 Oct 2020 10:13:13 +0200 Subject: [PATCH 01/10] Server build: force monorepo packages to be bundled --- client/webpack.config.node.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/client/webpack.config.node.js b/client/webpack.config.node.js index 9be998507071e..e195f44e0da47 100644 --- a/client/webpack.config.node.js +++ b/client/webpack.config.node.js @@ -52,13 +52,22 @@ function getExternals() { // with modules that are incompatible with webpack bundling. nodeExternals( { allowlist: [ - // `@automattic/components` is forced to be webpack-ed because it has SCSS and other - // non-JS asset imports that couldn't be processed by Node.js at runtime. - '@automattic/components', - - // The polyfills module is transpiled by Babel and only the `core-js` modules that are - // needed by current Node.js are included instead of the whole package. + '@automattic/calypso-analytics', '@automattic/calypso-polyfills', + '@automattic/components', + '@automattic/format-currency', + '@automattic/load-script', + '@automattic/popup-monitor', + '@automattic/react-i18n', + '@automattic/request-external-access', + '@automattic/social-previews', + '@automattic/tree-select', + '@automattic/viewport', + '@automattic/viewport-react', + 'i18n-calypso', + 'photon', + 'wpcom', + 'wpcom-proxy-request', /^core-js\//, // Ensure that file-loader files imported from packages in node_modules are From d759b04b4c1300c57a159f03e4e0f5a27751556c Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 2 Oct 2020 12:15:45 +0200 Subject: [PATCH 02/10] Add webpack plugin that finds all external modules and writes a file with the list --- client/server/bundler/external-modules.js | 56 +++++++++++++++++++++++ client/webpack.config.node.js | 2 + 2 files changed, 58 insertions(+) create mode 100644 client/server/bundler/external-modules.js diff --git a/client/server/bundler/external-modules.js b/client/server/bundler/external-modules.js new file mode 100644 index 0000000000000..da15ab393f430 --- /dev/null +++ b/client/server/bundler/external-modules.js @@ -0,0 +1,56 @@ +/** + * External Dependencies + */ +const fs = require( 'fs' ); +const path = require( 'path' ); +const { builtinModules } = require( 'module' ); + +function getModule( request ) { + const parts = request.split( '/' ); + if ( parts[ 0 ].startsWith( '@' ) ) { + return parts[ 0 ] + '/' + parts[ 1 ]; + } + return parts[ 0 ]; +} + +module.exports = class ExternalModulesWriter { + constructor( options ) { + this.options = { + path: './build', + filename: 'modules.json', + ...options, + }; + } + + apply( compiler ) { + compiler.hooks.afterEmit.tapAsync( 'ExternalModulesWriter', ( compilation ) => { + const externalModules = new Set(); + + for ( const module of compilation.modules ) { + if ( ! module.external ) { + continue; + } + + const requestModule = getModule( module.userRequest ); + + // native Node.js module, not in node_modules + if ( builtinModules.includes( requestModule ) ) { + continue; + } + + // loading local file by relative path, not in node_modules + if ( requestModule.startsWith( './' ) || requestModule.startsWith( '../' ) ) { + continue; + } + + externalModules.add( requestModule ); + } + + fs.writeFileSync( + path.join( this.options.path, this.options.filename ), + JSON.stringify( Array.from( externalModules ), null, 2 ), + 'utf8' + ); + } ); + } +}; diff --git a/client/webpack.config.node.js b/client/webpack.config.node.js index e195f44e0da47..08eb94486bc92 100644 --- a/client/webpack.config.node.js +++ b/client/webpack.config.node.js @@ -22,6 +22,7 @@ const FileConfig = require( '@automattic/calypso-build/webpack/file-loader' ); const { shouldTranspileDependency } = require( '@automattic/calypso-build/webpack/util' ); const nodeExternals = require( 'webpack-node-externals' ); const { BundleAnalyzerPlugin } = require( 'webpack-bundle-analyzer' ); +const ExternalModulesWriter = require( './server/bundler/external-modules' ); /** * Internal variables @@ -180,6 +181,7 @@ const webpackConfig = { 'components/empty-component' ), // Depends on BOM new webpack.IgnorePlugin( /^\.\/locale$/, /moment$/ ), // server doesn't use moment locales + ! isDevelopment && new ExternalModulesWriter(), ].filter( Boolean ), }; From 139486868b1f5ef6c3d3702d5034598988523197 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 2 Oct 2020 12:39:35 +0200 Subject: [PATCH 03/10] Add script that collects transitive external deps and ships them to build/ --- bin/copy-production-modules.js | 124 +++++++++++++++++++++++++++++++++ package.json | 3 + 2 files changed, 127 insertions(+) create mode 100644 bin/copy-production-modules.js diff --git a/bin/copy-production-modules.js b/bin/copy-production-modules.js new file mode 100644 index 0000000000000..4688ef013fc3f --- /dev/null +++ b/bin/copy-production-modules.js @@ -0,0 +1,124 @@ +const fs = require( 'fs' ); // eslint-disable-line import/no-nodejs-modules +const rcopy = require( 'recursive-copy' ); +const mkdirp = require( 'mkdirp' ); +const path = require( 'path' ); +const yargs = require( 'yargs' ); + +const args = yargs + .usage( 'Usage: $0' ) + .default( 'list', 'build/modules.json' ) + .default( 'dest', 'build' ) + .boolean( 'debug' ).argv; + +function debug( message ) { + args.debug && console.log( message ); +} + +try { + debug( 'reading modules from ' + args.list ); + const externalModules = JSON.parse( fs.readFileSync( args.list, 'utf8' ) ); + debug( 'bundle directly requests ' + externalModules.length + ' packages' ); + const shippingPkgList = processPackages( externalModules ); + shipDependencies( shippingPkgList ); +} catch ( err ) { + console.error( err ); + process.exit( 1 ); +} + +function processPackages( pkgList ) { + const context = { + pkgList: [], + visitedFolders: new Set(), + folderStack: [ '.' ], + requiredBy: 'the bundle', + depth: 0, + }; + + for ( const pkgName of pkgList ) { + processPackage( pkgName, context ); + } + + return context.pkgList; +} + +function processPackage( pkgName, context ) { + const { pkgList, folderStack, visitedFolders, requiredBy, depth } = context; + + const pkgFolder = folderStack[ 0 ] + '/node_modules/' + pkgName; + + // skip if the folder was already visited + if ( visitedFolders.has( pkgFolder ) ) { + return; + } + + // mark the folder as visited + visitedFolders.add( pkgFolder ); + + // Verify that the package resolves to a directory in `node_modules/`, and that + // it is not a symlink to a monorepo packages. Such packages couldn't be shipped and + // must be bundled. + const pkgRealpath = path.relative( '.', fs.realpathSync( pkgFolder ) ); + if ( ! pkgRealpath.startsWith( 'node_modules/' ) ) { + throw new Error( + 'package ' + pkgName + ' resolves outside node_modules/, likely a symlink: ' + pkgRealpath + ); + } + + // If the package is in the top-level folder, add it to the list of packages to ship. + // Subpackages are already shipped together with the parent. + if ( folderStack.length === 1 ) { + const indent = ' '.repeat( depth ); + debug( indent + 'shipping ' + pkgName + ': required by ' + requiredBy ); + pkgList.push( pkgName ); + } + + // read package's package.json + const pkgJson = JSON.parse( fs.readFileSync( pkgFolder + '/package.json' ) ); + + // collect dependencies from various fields + const depFields = [ 'dependencies', 'peerDependencies', 'optionalDependencies' ]; + const pkgDeps = depFields.flatMap( ( type ) => Object.keys( pkgJson[ type ] || {} ) ); + + // bail out if package has no dependencies + if ( ! pkgDeps.length ) { + return; + } + + // unshift the package's folder to the folder stack + const subFolderStack = [ pkgFolder, ...folderStack ]; + + // iterate its dependencies + for ( const depName of pkgDeps ) { + // Find the dependency in node_modules tree, starting with the package's `node_modules/` + // subdirectory and going up the directory tree to the root. + const foundFolderIdx = subFolderStack.findIndex( ( folder ) => + fs.existsSync( folder + '/node_modules/' + depName ) + ); + + if ( foundFolderIdx === -1 ) { + throw new Error( 'package not found: ' + depName + ', dependency of ' + pkgFolder ); + } + + // add the dependency to shipping list if eligible and recursively collect its dependencies + const subContext = { + ...context, + folderStack: subFolderStack.slice( foundFolderIdx ), + requiredBy: pkgName, + depth: depth + 1, + }; + processPackage( depName, subContext ); + } +} + +function shipDependencies( pkgList ) { + const destDir = path.join( args.dest, 'node_modules' ); + + debug( 'copying ' + pkgList.length + ' packages to ' + destDir ); + + mkdirp.sync( destDir ); + for ( const pkgName of pkgList ) { + rcopy( path.join( 'node_modules', pkgName ), path.join( destDir, pkgName ), { + overwrite: true, + } ); + } +} diff --git a/package.json b/package.json index 068cc1675e8b8..45e87ada727f7 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,8 @@ "build-static": "npx ncp static public", "prebuild-server": "mkdirp build", "build-server": "BROWSERSLIST_ENV=server webpack --config client/webpack.config.node.js --display errors-only", + "build-server:copy-modules": "node bin/copy-production-modules.js", + "postbuild-server": "node -e \"process.env.CALYPSO_ENV === 'production' && process.exit(1)\" || yarn run build-server:copy-modules", "build-server-stats": "NODE_ENV=production CALYPSO_ENV=production EMIT_STATS=true yarn run build-server", "build-client": "yarn run build-client-evergreen", "build-client-do": "node ${NODE_ARGS:---max-old-space-size=8192} ./node_modules/webpack/bin/webpack.js --config client/webpack.config.js --display errors-only", @@ -361,6 +363,7 @@ "react-dom": "^16.12.0", "react-test-renderer": "^16.12.0", "readline-sync": "^1.4.10", + "recursive-copy": "^2.0.10", "replace": "^1.1.5", "request": "^2.88.2", "rimraf": "^3.0.0", From 7434af342af2eb92eeb53a88fbff6c4bf3f9698c Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 16 Oct 2020 09:05:57 +0200 Subject: [PATCH 04/10] Remove redundant eslint-disable --- bin/copy-production-modules.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/copy-production-modules.js b/bin/copy-production-modules.js index 4688ef013fc3f..730b2b9d12d60 100644 --- a/bin/copy-production-modules.js +++ b/bin/copy-production-modules.js @@ -1,4 +1,4 @@ -const fs = require( 'fs' ); // eslint-disable-line import/no-nodejs-modules +const fs = require( 'fs' ); const rcopy = require( 'recursive-copy' ); const mkdirp = require( 'mkdirp' ); const path = require( 'path' ); From a404b133cfb15dbe7249b2d95c2f41d6f463dddf Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 16 Oct 2020 09:50:51 +0200 Subject: [PATCH 05/10] Use compiler.outputFileSystem to write the modules.json file --- client/server/bundler/external-modules.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/client/server/bundler/external-modules.js b/client/server/bundler/external-modules.js index da15ab393f430..e3a9115749ce3 100644 --- a/client/server/bundler/external-modules.js +++ b/client/server/bundler/external-modules.js @@ -1,8 +1,6 @@ /** * External Dependencies */ -const fs = require( 'fs' ); -const path = require( 'path' ); const { builtinModules } = require( 'module' ); function getModule( request ) { @@ -23,7 +21,7 @@ module.exports = class ExternalModulesWriter { } apply( compiler ) { - compiler.hooks.afterEmit.tapAsync( 'ExternalModulesWriter', ( compilation ) => { + compiler.hooks.emit.tapAsync( 'ExternalModulesWriter', ( compilation, callback ) => { const externalModules = new Set(); for ( const module of compilation.modules ) { @@ -46,11 +44,15 @@ module.exports = class ExternalModulesWriter { externalModules.add( requestModule ); } - fs.writeFileSync( - path.join( this.options.path, this.options.filename ), - JSON.stringify( Array.from( externalModules ), null, 2 ), - 'utf8' - ); + const json = JSON.stringify( Array.from( externalModules ), null, 2 ); + const { mkdirp, writeFile, join } = compiler.outputFileSystem; + const { path, filename } = this.options; + mkdirp( path, ( err ) => { + if ( err ) { + return callback( err ); + } + writeFile( join( path, filename ), json, 'utf8', callback ); + } ); } ); } }; From ccb58f3bd8784dca3628e1a072c00af8f49b7e3a Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Fri, 16 Oct 2020 10:45:19 +0200 Subject: [PATCH 06/10] Read the list of monorepo packages from the packages/ dir --- client/webpack.config.node.js | 36 +++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/client/webpack.config.node.js b/client/webpack.config.node.js index 08eb94486bc92..a39e8df8ccfb7 100644 --- a/client/webpack.config.node.js +++ b/client/webpack.config.node.js @@ -7,6 +7,8 @@ /** * External dependencies */ +const fs = require( 'fs' ); +const globby = require( 'globby' ); const path = require( 'path' ); const webpack = require( 'webpack' ); @@ -41,6 +43,16 @@ const fileLoader = FileConfig.loader( { const commitSha = process.env.hasOwnProperty( 'COMMIT_SHA' ) ? process.env.COMMIT_SHA : '(unknown)'; +function getMonorepoPackages() { + // find package.json files in all 1st level subdirectories of packages/ + return globby.sync( 'packages/*/package.json' ).map( ( pkgJsonPath ) => { + // read the package.json file + const pkgJson = JSON.parse( fs.readFileSync( pkgJsonPath ) ); + // create a package name regexp that matches all requests that start with it + return new RegExp( `^${ pkgJson.name }(/|$)` ); + } ); +} + /** * This lists modules that must use commonJS `require()`s * All modules listed here need to be ES5. @@ -53,22 +65,14 @@ function getExternals() { // with modules that are incompatible with webpack bundling. nodeExternals( { allowlist: [ - '@automattic/calypso-analytics', - '@automattic/calypso-polyfills', - '@automattic/components', - '@automattic/format-currency', - '@automattic/load-script', - '@automattic/popup-monitor', - '@automattic/react-i18n', - '@automattic/request-external-access', - '@automattic/social-previews', - '@automattic/tree-select', - '@automattic/viewport', - '@automattic/viewport-react', - 'i18n-calypso', - 'photon', - 'wpcom', - 'wpcom-proxy-request', + // Force all monorepo packages to be bundled. We can guarantee that they are safe + // to bundle, and can avoid shipping the entire contents of the `packages/` folder + // (there are symlinks into `packages/` from the `node_modules` folder) + ...getMonorepoPackages(), + + // bundle the core-js polyfills. We pick only a very small subset of the library + // to polyfill a few things that are not supported by the latest LTS Node.js, + // and this avoids shipping the entire library which is fairly big. /^core-js\//, // Ensure that file-loader files imported from packages in node_modules are From 577a8f2c475b727ffb8860d95edb206e0c4ea5c9 Mon Sep 17 00:00:00 2001 From: Sergio Cinos Date: Thu, 22 Oct 2020 16:18:43 +0200 Subject: [PATCH 07/10] Copy only required folders to run the server --- Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 293959d74539c..d696f9f51c437 100644 --- a/Dockerfile +++ b/Dockerfile @@ -47,7 +47,9 @@ ENV NODE_ENV production WORKDIR /calypso RUN apk add --no-cache tini -COPY --from=builder --chown=nobody:nobody /calypso/ /calypso/ +COPY --from=builder --chown=nobody:nobody /calypso/build /calypso/build +COPY --from=builder --chown=nobody:nobody /calypso/public /calypso/public +COPY --from=builder --chown=nobody:nobody /calypso/config /calypso/config USER nobody ENTRYPOINT ["/sbin/tini", "--"] From 018b30e22f1f1dbb6e228ce4bcf788fc7d903abf Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 26 Oct 2020 08:49:04 +0100 Subject: [PATCH 08/10] Copy service-worker.js to public/ dir and make express.static serve it from there --- client/server/pwa/index.js | 4 +--- package.json | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/client/server/pwa/index.js b/client/server/pwa/index.js index d4b6558c5b2f4..089da3b28a67c 100644 --- a/client/server/pwa/index.js +++ b/client/server/pwa/index.js @@ -18,9 +18,7 @@ export default () => { // service-worker needs to be served from root to avoid scope issues app.use( '/service-worker.js', - express.static( - path.resolve( __dirname, '..', '..', 'lib', 'service-worker', 'service-worker.js' ) - ) + express.static( path.resolve( __dirname, '..', '..', '..', 'public', 'service-worker.js' ) ) ); return app; diff --git a/package.json b/package.json index 45e87ada727f7..adef6a6d45688 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "build-desktop:app": "cd desktop && yarn run build", "test-desktop:e2e": "node desktop/e2e/run.js", "build-docker": "node bin/build-docker.js", - "build-static": "npx ncp static public", + "build-static": "npx ncp static public && npx ncp client/lib/service-worker public --filter=\"service-worker(/service-worker\\.js)?$\"", "prebuild-server": "mkdirp build", "build-server": "BROWSERSLIST_ENV=server webpack --config client/webpack.config.node.js --display errors-only", "build-server:copy-modules": "node bin/copy-production-modules.js", From ac9000f820a1a26b4e11e4b76c818433c62950dc Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 26 Oct 2020 09:25:52 +0100 Subject: [PATCH 09/10] Disable service-worker on Desktop --- client/server/boot/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/client/server/boot/index.js b/client/server/boot/index.js index 57439ec992080..2d76816161933 100644 --- a/client/server/boot/index.js +++ b/client/server/boot/index.js @@ -71,7 +71,9 @@ export default function setup() { } } - app.use( pwa() ); + if ( ! config.isEnabled( 'desktop' ) ) { + app.use( pwa() ); + } // attach the static file server to serve the `public` dir app.use( '/calypso', express.static( path.resolve( __dirname, '..', '..', '..', 'public' ) ) ); From 9b57b03a1311193034f7cac3a1d0a126f1848491 Mon Sep 17 00:00:00 2001 From: Jarda Snajdr Date: Mon, 26 Oct 2020 10:32:16 +0100 Subject: [PATCH 10/10] Fix relative import paths in server/boot --- client/server/boot/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/client/server/boot/index.js b/client/server/boot/index.js index 2d76816161933..f8dc2e88d2da4 100644 --- a/client/server/boot/index.js +++ b/client/server/boot/index.js @@ -10,12 +10,12 @@ import userAgent from 'express-useragent'; /** * Internal dependencies */ -import analytics from 'server/lib/analytics'; -import config from 'server/config'; -import api from 'server/api'; -import pages from 'server/pages'; -import pwa from 'server/pwa'; -import loggerMiddleware from 'server/middleware/logger'; +import analytics from 'calypso/server/lib/analytics'; +import config from 'calypso/server/config'; +import api from 'calypso/server/api'; +import pages from 'calypso/server/pages'; +import pwa from 'calypso/server/pwa'; +import loggerMiddleware from 'calypso/server/middleware/logger'; /** * Returns the server HTTP request handler "app". @@ -33,7 +33,7 @@ export default function setup() { app.use( loggerMiddleware() ); if ( 'development' === process.env.NODE_ENV ) { - require( 'server/bundler' )( app ); + require( 'calypso/server/bundler' )( app ); if ( config.isEnabled( 'wpcom-user-bootstrap' ) ) { if ( config( 'wordpress_logged_in_cookie' ) ) { @@ -92,7 +92,7 @@ export default function setup() { } ); if ( config.isEnabled( 'devdocs' ) ) { - app.use( require( 'server/devdocs' ).default() ); + app.use( require( 'calypso/server/devdocs' ).default() ); } if ( config.isEnabled( 'desktop' ) ) {