Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: compile out remaining typescript; add tsc type checking via jsdocs #3747

Merged
merged 12 commits into from
Nov 21, 2017
1 change: 0 additions & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ test_script:
cache:
#- chrome-win32 -> appveyor.yml,package.json
- node_modules -> appveyor.yml,package.json,yarn.lock
- lighthouse-cli\node_modules -> appveyor.yml,package.json,yarn.lock,lighthouse-cli\package.json,lighthouse-cli\yarn.lock
- lighthouse-extension\node_modules -> appveyor.yml,package.json,yarn.lock,lighthouse-extension\package.json,lighthouse-extension\yarn.lock
- lighthouse-viewer\node_modules -> appveyor.yml,package.json,yarn.lock,lighthouse-viewer\package.json,lighthouse-viewer\yarn.lock
- '%LOCALAPPDATA%\Yarn -> appveyor.yml,package.json,yarn.lock'
7 changes: 0 additions & 7 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,9 @@
**/dist/**
lighthouse-extension/app/scripts


**/closure/*/*
coverage/**

# Typescript compiled
!lighthouse-cli/index.js
lighthouse-cli/*.js
lighthouse-cli/commands/*.js
lighthouse-cli/types/*.js

# Generated files
plots/out*

Expand Down
10 changes: 0 additions & 10 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,6 @@ last-run-results.html

closure-error.log

!lighthouse-cli/index.js
lighthouse-cli/*.map
lighthouse-cli/*.js

lighthouse-cli/commands/*.map
lighthouse-cli/commands/*.js

lighthouse-cli/types/*.js
lighthouse-cli/types/*.map

/chrome-linux/
/chrome-win32/
/chrome.zip
Expand Down
9 changes: 6 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@ cache:
yarn: true
directories:
- node_modules
- lighthouse-cli/node_modules
- lighthouse-extension/node_modules
- lighthouse-viewer/node_modules
- /home/travis/.rvm/gems/
before_install:
# TODO: remove after yarn is 1.0+ on Travis: https://github.com/travis-ci/travis-ci/issues/7566
- curl -o- -L https://yarnpkg.com/install.sh | bash
- export PATH="$HOME/.yarn/bin:$PATH"
install:
- yarn
# travis can't handle the parallel install (without caches)
Expand All @@ -31,9 +34,9 @@ before_script:
- yarn build-all
script:
- yarn bundlesize
- yarn test-cli-formatting
- yarn lint
- yarn unit
- yarn type
- yarn closure
- yarn smoke
- yarn smokehouse
Expand All @@ -45,7 +48,7 @@ before_cache:
- rm -rf ./node_modules/temp-devtoolsfrontend/
- rm -rf ./node_modules/temp-devtoolsprotocol/
after_success:
- ./lighthouse-core/scripts/generate-combined-coverage.sh
- yarn coveralls
after_failure:
- grep 'No screenshots' perf.json && travis-artifacts upload --path perf-0.trace.json
addons:
Expand Down
25 changes: 0 additions & 25 deletions jsconfig.json

This file was deleted.

6 changes: 0 additions & 6 deletions lighthouse-cli/.clang-format

This file was deleted.

22 changes: 0 additions & 22 deletions lighthouse-cli/README.md

This file was deleted.

90 changes: 54 additions & 36 deletions lighthouse-cli/bin.ts → lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,51 +5,57 @@
*/
'use strict';

import {existsSync} from 'fs';
import * as path from 'path';
const existsSync = require('fs').existsSync;
const path = require('path');

import * as Commands from './commands/commands';
import * as Printer from './printer';
import {getFlags} from './cli-flags';
import {runLighthouse} from './run';
const commands = require('./commands/commands.js');
const printer = require('./printer.js');
const getFlags = require('./cli-flags.js').getFlags;
const runLighthouse = require('./run').runLighthouse;

const log = require('lighthouse-logger');
// @ts-ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason typescript won't resolve json files as part of require() (you get a "Cannot find module '../lighthouse-core/config/perf.json'" here). Searching didn't find a better solution, and @JoelEinbinder did this in puppeteer, so I'm going with that unless anyone has a better fix :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did some searching as well, there is no support.

const perfOnlyConfig = require('../lighthouse-core/config/perf.json');
// @ts-ignore
const pkg = require('../package.json');
const Sentry = require('../lighthouse-core/lib/sentry');

// accept noop modules for these, so the real dependency is optional.
import {updateNotifier} from './shim-modules';
import {askPermission} from './sentry-prompt';
const updateNotifier = require('update-notifier');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentionally not shimming anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentionally not shimming anymore?

yeah, I think(?) it only added value to users not installing from package.json and managing all dependencies themselves (since we didn't add them as peerDependencies), and I don't think we have any of those users anymore, so it seemed like a good time to remove. No big deal to add back if we want to keep though

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that seems fine, better to be clear about our non users than half support :)

const askPermission = require('./sentry-prompt').askPermission;

/**
* @return {boolean}
*/
function isDev() {
return existsSync(path.join(__dirname, '../.git'));
}

// Tell user if there's a newer version of LH.
updateNotifier({pkg}).notify();

const cliFlags = getFlags();
const /** @type {!LH.Flags} */ cliFlags = getFlags();

// Process terminating command
if (cliFlags.listAllAudits) {
Commands.ListAudits();
commands.listAudits();
}

// Process terminating command
if (cliFlags.listTraceCategories) {
Commands.ListTraceCategories();
commands.listTraceCategories();
}

/** @type {string} */
const url = cliFlags._[0];

let config: Object|null = null;
/** @type {!LH.Config|undefined} */
let config;
if (cliFlags.configPath) {
// Resolve the config file path relative to where cli was called.
cliFlags.configPath = path.resolve(process.cwd(), cliFlags.configPath);
config = require(cliFlags.configPath);
config = /** @type {!LH.Config} */ (require(cliFlags.configPath));
} else if (cliFlags.perf) {
config = perfOnlyConfig;
config = /** @type {!LH.Config} */ (perfOnlyConfig);
}

// set logging preferences
Expand All @@ -61,27 +67,39 @@ if (cliFlags.verbose) {
}
log.setLevel(cliFlags.logLevel);

if (cliFlags.output === Printer.OutputMode[Printer.OutputMode.json] && !cliFlags.outputPath) {
if (cliFlags.output === printer.OutputMode.json && !cliFlags.outputPath) {
cliFlags.outputPath = 'stdout';
}

export async function run() {
if (typeof cliFlags.enableErrorReporting === 'undefined') {
cliFlags.enableErrorReporting = await askPermission();
}

Sentry.init({
url,
flags: cliFlags,
environmentData: {
name: 'redacted', // prevent sentry from using hostname
environment: isDev() ? 'development' : 'production',
release: pkg.version,
tags: {
channel: 'cli',
},
},
});

return runLighthouse(url, cliFlags, config);
/**
* @return {!Promise<(void|!LH.Results)>}
*/
function run() {
return Promise.resolve()
.then(_ => {
if (typeof cliFlags.enableErrorReporting === 'undefined') {
return askPermission().then(answer => {
cliFlags.enableErrorReporting = answer;
});
}
})
.then(_ => {
Sentry.init({
url,
flags: cliFlags,
environmentData: {
name: 'redacted', // prevent sentry from using hostname
environment: isDev() ? 'development' : 'production',
release: pkg.version,
tags: {
channel: 'cli',
},
},
});

return runLighthouse(url, cliFlags, config);
});
}

module.exports = {
run,
};
56 changes: 30 additions & 26 deletions lighthouse-cli/cli-flags.ts → lighthouse-cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@
*/
'use strict';

/* eslint-disable max-len */

const yargs = require('yargs');
// @ts-ignore
const pkg = require('../package.json');
const Driver = require('../lighthouse-core/gather/driver.js');
const printer = require('./printer');

import {GetValidOutputOptions, OutputMode} from './printer';

export interface Flags {
port: number, chromeFlags: string, output: any, outputPath: string, saveArtifacts: boolean,
saveAssets: boolean, view: boolean, maxWaitForLoad: number, logLevel: string,
hostname: string, blockedUrlPatterns: string[], enableErrorReporting: boolean
}

export function getFlags(manualArgv?: string) {
/**
* @param {string=} manualArgv
* @return {!LH.Flags}
*/
function getFlags(manualArgv) {
// @ts-ignore yargs() is incorrectly typed as not returning itself
const y = manualArgv ? yargs(manualArgv) : yargs;

return y.help('help')
.version(() => pkg.version)
.showHelpOnFail(false, 'Specify --help for available options')
Expand Down Expand Up @@ -47,16 +47,16 @@ export function getFlags(manualArgv?: string) {
.group(['verbose', 'quiet'], 'Logging:')
.describe({
verbose: 'Displays verbose logging',
quiet: 'Displays no progress, debug logs or errors'
quiet: 'Displays no progress, debug logs or errors',
})

.group(
[
'save-assets', 'save-artifacts', 'list-all-audits', 'list-trace-categories',
'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port',
'hostname', 'max-wait-for-load', 'enable-error-reporting'
],
'Configuration:')
[
'save-assets', 'save-artifacts', 'list-all-audits', 'list-trace-categories',
'additional-trace-categories', 'config-path', 'chrome-flags', 'perf', 'port',
'hostname', 'max-wait-for-load', 'enable-error-reporting',
],
'Configuration:')
.describe({
'enable-error-reporting':
'Enables error reporting (prompts once by default, setting this flag will force error reporting to that state).',
Expand Down Expand Up @@ -90,31 +90,31 @@ export function getFlags(manualArgv?: string) {
.describe({
'output': `Reporter for the results, supports multiple values`,
'output-path': `The file path to output the results. Use 'stdout' to write to stdout.
If using JSON output, default is stdout.
If using HTML output, default is a file in the working directory with a name based on the test URL and date.
If using multiple outputs, --output-path is ignored.
Example: --output-path=./lighthouse-results.html`,
'view': 'Open HTML report in your browser'
If using JSON output, default is stdout.
If using HTML output, default is a file in the working directory with a name based on the test URL and date.
If using multiple outputs, --output-path is ignored.
Example: --output-path=./lighthouse-results.html`,
'view': 'Open HTML report in your browser',
})

// boolean values
.boolean([
'disable-storage-reset', 'disable-device-emulation', 'disable-cpu-throttling',
'disable-network-throttling', 'save-assets', 'save-artifacts', 'list-all-audits',
'list-trace-categories', 'perf', 'view', 'verbose', 'quiet', 'help'
'list-trace-categories', 'perf', 'view', 'verbose', 'quiet', 'help',
])
.choices('output', GetValidOutputOptions())
.choices('output', printer.getValidOutputOptions())
// force as an array
.array('blocked-url-patterns')

// default values
.default('chrome-flags', '')
.default('disable-cpu-throttling', false)
.default('output', GetValidOutputOptions()[OutputMode.domhtml])
.default('output', 'domhtml')
.default('port', 0)
.default('hostname', 'localhost')
.default('max-wait-for-load', Driver.MAX_WAIT_FOR_FULLY_LOADED)
.check((argv: {listAllAudits?: boolean, listTraceCategories?: boolean, _: Array<any>}) => {
.check(/** @param {!LH.Flags} argv */ (argv) => {
// Make sure lighthouse has been passed a url, or at least one of --list-all-audits
// or --list-trace-categories. If not, stop the program and ask for a url
if (!argv.listAllAudits && !argv.listTraceCategories && argv._.length === 0) {
Expand All @@ -128,3 +128,7 @@ Example: --output-path=./lighthouse-results.html`,
.wrap(yargs.terminalWidth())
.argv;
}

module.exports = {
getFlags,
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
* 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.
*/
'use strict';

import ListAudits from './list-audits';
import ListTraceCategories from './list-trace-categories';
const listAudits = require('./list-audits.js');
const listTraceCategories = require('./list-trace-categories.js');

export {ListAudits, ListTraceCategories}
module.exports = {
listAudits,
listTraceCategories,
};
Loading