From 86b6d2c99d9c45dba58bc38ec80e97bb1c5ab9a3 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Fri, 24 Dec 2021 02:15:25 -0800 Subject: [PATCH] Make flipper-server NPX-able Summary: Make sure flipper-server is bundled in such a way that it is self-contained NPX-able. Also added some checks to make sure that dev dependencies don't accidentallly end up in in Flipper buidls Reviewed By: nikoant Differential Revision: D33190254 fbshipit-source-id: 443162e537d8ca9f956acac2d7bd52cbf0c92172 --- .../src/electron-requires.ts | 5 ++ ...form-server.ts => transform-server-dev.ts} | 2 + .../src/transform-server-prod.ts | 46 +++++++++++++++ .../src/utils/environmentInfo.tsx | 1 + desktop/flipper-server/README.md | 11 ++++ desktop/flipper-server/package.json | 14 +++-- .../flipper-server/src/startWebServerDev.tsx | 10 ++-- desktop/scripts/build-utils.ts | 58 ++++++++++++++----- 8 files changed, 123 insertions(+), 24 deletions(-) rename desktop/babel-transformer/src/{transform-server.ts => transform-server-dev.ts} (88%) create mode 100644 desktop/babel-transformer/src/transform-server-prod.ts diff --git a/desktop/babel-transformer/src/electron-requires.ts b/desktop/babel-transformer/src/electron-requires.ts index e32a80fc0..96bf71990 100644 --- a/desktop/babel-transformer/src/electron-requires.ts +++ b/desktop/babel-transformer/src/electron-requires.ts @@ -45,7 +45,12 @@ const BUILTINS = [ 'v8', 'repl', 'timers', + // MWE node-fetch looks strange here, not sure what the effect of changing that would be 'node-fetch', + // jest is referred to in source code, like in TestUtils, but we don't want to ever bundle it up! + 'jest', + '@testing-library/react', + '@testing-library/dom', ]; const IGNORED_MODULES = [ diff --git a/desktop/babel-transformer/src/transform-server.ts b/desktop/babel-transformer/src/transform-server-dev.ts similarity index 88% rename from desktop/babel-transformer/src/transform-server.ts rename to desktop/babel-transformer/src/transform-server-dev.ts index d8ae4abcd..425019fb9 100644 --- a/desktop/babel-transformer/src/transform-server.ts +++ b/desktop/babel-transformer/src/transform-server-dev.ts @@ -20,6 +20,8 @@ const presets = [ }, ], ]; + +// In DEV builds, we keep node_modules as is, as to not waste resources on trying to bundle them const plugins = [require('./electron-requires-server'), require('./fb-stubs')]; module.exports = { diff --git a/desktop/babel-transformer/src/transform-server-prod.ts b/desktop/babel-transformer/src/transform-server-prod.ts new file mode 100644 index 000000000..a7edc4c4c --- /dev/null +++ b/desktop/babel-transformer/src/transform-server-prod.ts @@ -0,0 +1,46 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {default as doTransform} from './transform'; +import {default as getCacheKey} from './get-cache-key'; + +const presets = [ + [ + '@babel/preset-env', + { + targets: { + node: 'current', + }, + }, + ], +]; + +// In PROD builds, we bundle up all node_modules as well, so that there is a single JS file to run in the end, without +// needing to install deps for either flipper-server or flipper-server-core. +// This is also the reason that all server deps are DEV deps +// electron-requires makes sure that *only* requires of built in node_modules are using "electronRequire" +// (which effectively makes them external, as electronRequire === require, but not rolled up with Metro) +const plugins = [require('./electron-requires'), require('./fb-stubs')]; + +module.exports = { + transform, + getCacheKey, +}; + +function transform({ + filename, + options, + src, +}: { + filename: string; + options: any; + src: string; +}) { + return doTransform({filename, options, src, presets, plugins}); +} diff --git a/desktop/flipper-server-core/src/utils/environmentInfo.tsx b/desktop/flipper-server-core/src/utils/environmentInfo.tsx index be08ec1a5..27ea6dbef 100644 --- a/desktop/flipper-server-core/src/utils/environmentInfo.tsx +++ b/desktop/flipper-server-core/src/utils/environmentInfo.tsx @@ -7,6 +7,7 @@ * @format */ +import process from 'process'; import os from 'os'; import fs from 'fs-extra'; import path from 'path'; diff --git a/desktop/flipper-server/README.md b/desktop/flipper-server/README.md index 98e60086a..5768822ff 100644 --- a/desktop/flipper-server/README.md +++ b/desktop/flipper-server/README.md @@ -27,3 +27,14 @@ yarn build:flipper-server ``` Pass the `--open` flag to open Flipper server after building + +Use `--no-rebuild-plugins` to speed up subsequent builds if default plugins have been build already + +### Test NPX build + +``` +cd /desktop/ +yarn build:flipper-server +cd flipper-server +yarn test:npx +``` diff --git a/desktop/flipper-server/package.json b/desktop/flipper-server/package.json index 1f68f2dbe..10603ae2d 100644 --- a/desktop/flipper-server/package.json +++ b/desktop/flipper-server/package.json @@ -9,29 +9,31 @@ "flipperBundlerEntry": "src", "license": "MIT", "bugs": "https://github.com/facebook/flipper/issues", + "dependenciesComment": "mac-ca is required dynamically for darwin, node-fetch is treated special in electron-requires, not sure why", "dependencies": { - "chalk": "^4.1.2", - "express": "^4.15.2", - "fs-extra": "^9.0.0", "mac-ca": "^1.0.6", - "p-filter": "^2.1.0", - "socket.io": "^4.3.1" + "node-fetch": "^2.6.6" }, "devDependencies": { "@types/express": "^4.17.13", "@types/node": "^15.12.5", + "chalk": "^4.1.2", + "express": "^4.15.2", "flipper-common": "0.0.0", "flipper-pkg-lib": "0.0.0", "flipper-server-core": "0.0.0", + "fs-extra": "^9.0.0", "metro": "^0.66.2", "open": "^8.3.0", + "p-filter": "^2.1.0", + "socket.io": "^4.3.1", "yargs": "^17.0.1" }, "peerDependencies": {}, "scripts": { "reset": "rimraf lib *.tsbuildinfo", "build": "tsc -b", - "prepack": "yarn reset && yarn build" + "test:npx": "yarn pack && cd ~/Desktop && rm -rf ~/.npm/_npx/ && mv ~/fbsource/xplat/sonar/desktop/flipper-server/flipper-server-v0.0.0.tgz . && npx flipper-server-v0.0.0.tgz" }, "files": [ "dist/**/*", diff --git a/desktop/flipper-server/src/startWebServerDev.tsx b/desktop/flipper-server/src/startWebServerDev.tsx index cebe01ed8..85a117ced 100644 --- a/desktop/flipper-server/src/startWebServerDev.tsx +++ b/desktop/flipper-server/src/startWebServerDev.tsx @@ -13,12 +13,7 @@ import http from 'http'; import path from 'path'; import fs from 'fs-extra'; import socketio from 'socket.io'; -import {getWatchFolders} from 'flipper-pkg-lib'; -import Metro from 'metro'; import pFilter from 'p-filter'; -// provided by Metro -// eslint-disable-next-line -import MetroResolver from 'metro-resolver'; import {homedir} from 'os'; // This file is heavily inspired by scripts/start-dev-server.ts! @@ -59,6 +54,11 @@ export async function startWebServerDev( socket: socketio.Server, rootDir: string, ) { + // prevent bundling! + const Metro = electronRequire('metro'); + const MetroResolver = electronRequire('metro-resolver'); + const {getWatchFolders} = electronRequire('flipper-pkg-lib'); + const babelTransformationsDir = path.resolve( rootDir, 'babel-transformer', diff --git a/desktop/scripts/build-utils.ts b/desktop/scripts/build-utils.ts index c2836315d..2f13b47cd 100644 --- a/desktop/scripts/build-utils.ts +++ b/desktop/scripts/build-utils.ts @@ -394,13 +394,18 @@ export async function compileServerMain(dev: boolean) { transformer: { babelTransformerPath: path.join( babelTransformationsDir, - 'transform-server', + 'transform-server-' + (dev ? 'dev' : 'prod'), ), ...minifierConfig, }, resolver: { - sourceExts: ['tsx', 'ts', 'js', 'json'], - resolverMainFields: ['flipperBundlerEntry', 'module', 'main'], + // no 'mjs' / 'module'; it caused issues + sourceExts: ['tsx', 'ts', 'js', 'json', 'cjs'], + resolverMainFields: ['flipperBundlerEntry', 'main'], + resolveRequest(context: any, moduleName: string, ...rest: any[]) { + assertSaneImport(context, moduleName); + return defaultResolve(context, moduleName, ...rest); + }, }, }); await Metro.runBuild(config, { @@ -408,7 +413,7 @@ export async function compileServerMain(dev: boolean) { entry: path.join(serverDir, 'src', 'index.tsx'), out, dev, - minify: !dev, + minify: false, // !dev, sourceMap: true, sourceMapUrl: dev ? 'index.map' : undefined, inlineSourceMap: false, @@ -468,8 +473,9 @@ export async function buildBrowserBundle(dev: boolean) { blacklistRE: [/\.native\.js$/], sourceExts: ['js', 'jsx', 'ts', 'tsx', 'json', 'mjs', 'cjs'], resolveRequest(context: any, moduleName: string, ...rest: any[]) { + assertSaneImport(context, moduleName); // flipper is special cased, for plugins that we bundle, - // we want to resolve `impoSrt from 'flipper'` to 'flipper-ui-core', which + // we want to resolve `import from 'flipper'` to 'flipper-ui-core', which // defines all the deprecated exports if (moduleName === 'flipper') { return MetroResolver.resolve(context, 'flipper-ui-core', ...rest); @@ -482,14 +488,7 @@ export async function buildBrowserBundle(dev: boolean) { type: 'empty', }; } - return MetroResolver.resolve( - { - ...context, - resolveRequest: null, - }, - moduleName, - ...rest, - ); + return defaultResolve(context, moduleName, ...rest); }, }, }); @@ -546,3 +545,36 @@ export async function launchServer(startBundler: boolean, open: boolean) { }, ); } + +function assertSaneImport(context: any, moduleName: string) { + // This function checks that we aren't accidentally bundling up something huge we don't want to + // bundle up + if ( + moduleName.startsWith('jest') || + (moduleName.startsWith('metro') && + !moduleName.startsWith('metro-runtime')) || + moduleName === 'Metro' || + moduleName.startsWith('babel') || + moduleName.startsWith('typescript') || + moduleName.startsWith('electron') || + moduleName.startsWith('@testing-library') + ) { + console.error( + `Found a reference to module '${moduleName}', which should not be imported / required. Referer: ${context.originModulePath}`, + ); + // throwing errors doesn't really stop Metro :-/ + process.exit(1); + } +} + +function defaultResolve(...rest: any[]) { + const [context, moduleName] = rest; + return MetroResolver.resolve( + { + ...context, + resolveRequest: null, + }, + moduleName, + ...rest, + ); +}