From 20c22ad9adfb00ef7a5c05c2a67510f1068d5edb Mon Sep 17 00:00:00 2001 From: Jack Franklin Date: Fri, 17 Apr 2020 10:29:40 +0100 Subject: [PATCH] chore: fix installing from GitHub URL (#5669) The change to the install script to require TypeScript works fine when installing from npm (because on npm the `lib` directory with the compiled code already exists) but doesn't if you install from a GitHub URL. By default it seems npm uses the `files` list when you install from GitHub which means it's missing a bunch of files that we need to compile. Additionally by default when installing from a GitHub URL npm doesn't install the dependencies which is an issue for us when we need to compile TypeScript. The fix is to create a `prepare` script that runs TypeScript if required. From the npm docs [1]: > `prepare`: Run both BEFORE the package is packed and published, on > local npm install without any arguments, and when installing git > dependencies And from the npm docs on install [2], it confirms that if a package has a `prepare` script it is run when installing from GitHub: > As with regular git dependencies, dependencies and devDependencies > will be installed if the package has a prepare script, before the > package is done installing. Despite having the `prepare` script we still need the TypeScript check in `install.js` to satisfy the 3rd scenario below where we need to force a compile: * If I'm a user installing `puppeteer@X` from npm, the module is published with the `lib/` directory of compiled code, so I'm set. * If I'm a user installing Puppeteer from GitHub, the `prepare` script will run TypeScript for me so I'm set. * If I'm a developer working on Puppeteer, the `prepare` script also runs but _after_ `npm install` which means `install.js` fails as it requires `./lib/helper.js`. So in `install.js` we call `compileTypeScriptIfRequired` to catch this case. [1]: https://docs.npmjs.com/misc/scripts [2]: https://docs.npmjs.com/cli/install Co-authored-by: Mathias Bynens Fixes #5660. --- install.js | 44 +++-------------------------- package.json | 1 + typescript-if-required.js | 59 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 40 deletions(-) create mode 100644 typescript-if-required.js diff --git a/install.js b/install.js index 114bc7dc1144b..8555af326dc0a 100644 --- a/install.js +++ b/install.js @@ -14,45 +14,6 @@ * limitations under the License. */ -const fs = require('fs'); -const path = require('path'); -const child_process = require('child_process'); -const {promisify} = require('util'); - -const fsAccess = promisify(fs.access); -const exec = promisify(child_process.exec); - -const fileExists = async filePath => fsAccess(filePath).then(() => true).catch(() => false); - -/* - * Now Puppeteer is built with TypeScript, we need to ensure that - * locally we have the generated output before trying to install. - * - * For users installing puppeteer this is fine, they will have the - * generated lib/ directory as we ship it when we publish to npm. - * - * However, if you're cloning the repo to contribute, you won't have the - * generated lib/ directory so this script checks if we need to run - * TypeScript first to ensure the output exists and is in the right - * place. - */ -async function compileTypeScript() { - return exec('npm run tsc').catch(err => { - console.error('Error running TypeScript', err); - process.exit(1); - }); -} - -async function ensureLibDirectoryExists() { - const libPath = path.join(__dirname, 'lib'); - const libExists = await fileExists(libPath); - if (libExists) return; - - logPolitely('Compiling TypeScript before install...'); - await compileTypeScript(); -} - - /** * This file is part of public API. * @@ -62,13 +23,16 @@ async function ensureLibDirectoryExists() { * still possible to install a supported browser using this script when * necessary. */ + +const compileTypeScriptIfRequired = require('./typescript-if-required'); + const supportedProducts = { 'chrome': 'Chromium', 'firefox': 'Firefox Nightly' }; async function download() { - await ensureLibDirectoryExists(); + await compileTypeScriptIfRequired(); const downloadHost = process.env.PUPPETEER_DOWNLOAD_HOST || process.env.npm_config_puppeteer_download_host || process.env.npm_package_config_puppeteer_download_host; const puppeteer = require('./index'); diff --git a/package.json b/package.json index 205e77cc0a917..26419672b4ae9 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "debug-unit": "node --inspect-brk test/test.js", "test-doclint": "mocha --config mocha-config/doclint-tests.js", "test": "npm run tsc && npm run lint --silent && npm run coverage && npm run test-doclint && npm run test-types", + "prepare": "node typescript-if-required.js", "prepublishOnly": "npm run tsc", "dev-install": "npm run tsc && node install.js", "install": "node install.js", diff --git a/typescript-if-required.js b/typescript-if-required.js new file mode 100644 index 0000000000000..4b3a3406d72c5 --- /dev/null +++ b/typescript-if-required.js @@ -0,0 +1,59 @@ +/** + * Copyright 2020 Google Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const child_process = require('child_process'); +const path = require('path'); +const fs = require('fs'); +const {promisify} = require('util'); + +const exec = promisify(child_process.exec); +const fsAccess = promisify(fs.access); + +const fileExists = async filePath => fsAccess(filePath).then(() => true).catch(() => false); +/* + + * Now Puppeteer is built with TypeScript, we need to ensure that + * locally we have the generated output before trying to install. + * + * For users installing puppeteer this is fine, they will have the + * generated lib/ directory as we ship it when we publish to npm. + * + * However, if you're cloning the repo to contribute, you won't have the + * generated lib/ directory so this script checks if we need to run + * TypeScript first to ensure the output exists and is in the right + * place. + */ +async function compileTypeScript() { + return exec('npm run tsc').catch(err => { + console.error('Error running TypeScript', err); + process.exit(1); + }); +} + +async function compileTypeScriptIfRequired() { + const libPath = path.join(__dirname, 'lib'); + const libExists = await fileExists(libPath); + if (libExists) return; + + console.log('Puppeteer:', 'Compiling TypeScript...'); + await compileTypeScript(); +} + +// It's being run as node typescript-if-required.js, not require('..') +if (require.main === module) + compileTypeScriptIfRequired(); + +module.exports = compileTypeScriptIfRequired;