From ff924bd8d0b66e6dfa8485ef1c3343967e0e5c18 Mon Sep 17 00:00:00 2001 From: Justin Fagnani Date: Mon, 7 Nov 2016 17:07:30 -0800 Subject: [PATCH 1/2] Enable strictNullChecks --- src/args.ts | 2 +- src/build/build.ts | 2 +- src/build/prefetch.ts | 25 ++++++++++++++----------- src/build/sw-precache.ts | 6 +++--- src/commands/build.ts | 2 +- src/commands/lint.ts | 2 +- src/commands/serve.ts | 2 +- src/environments/environments.ts | 5 +++-- src/github/github.ts | 10 +++++----- src/init/init.ts | 6 ++++-- src/polymer-cli.ts | 7 ++++--- tsconfig.json | 1 + 12 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/args.ts b/src/args.ts index 6b57bbfc..c152b162 100644 --- a/src/args.ts +++ b/src/args.ts @@ -17,7 +17,7 @@ export const globalArguments: ArgDescriptor[] = [ name: 'env', description: 'The environment to use to specialize certain commands, ' + 'like build', - type(value: string): Environment { + type(value: string): Environment|undefined { return buildEnvironment(value); }, group: 'global', diff --git a/src/build/build.ts b/src/build/build.ts index db116049..851d2e31 100644 --- a/src/build/build.ts +++ b/src/build/build.ts @@ -91,7 +91,7 @@ export async function build(options: BuildOptions, config: ProjectConfig): Promi .once('data', () => { logger.info('Generating build/unbundled...'); }) .pipe( gulpif( - options.insertDependencyLinks, + options.insertDependencyLinks || false, new PrefetchTransform(polymerProject) ) ) diff --git a/src/build/prefetch.ts b/src/build/prefetch.ts index ebee3048..d0d442ce 100644 --- a/src/build/prefetch.ts +++ b/src/build/prefetch.ts @@ -42,8 +42,9 @@ export class PrefetchTransform extends Transform { type: 'import' | 'prefetch' ) { let contents = file.contents.toString(); - let ast = parse5.parse(contents); - let head = dom5.query(ast, dom5.predicates.hasTagName('head')); + const ast = parse5.parse(contents); + // parse5 always produces a element + const head = dom5.query(ast, dom5.predicates.hasTagName('head'))!; for (let dep of deps) { if (dep.startsWith(this.config.root)) { dep = path.relative(file.dirname, dep); @@ -52,7 +53,7 @@ export class PrefetchTransform extends Transform { if (type === 'prefetch') { dep = path.join('/', dep); } - let link = dom5.constructors.element('link'); + const link = dom5.constructors.element('link'); dom5.setAttribute(link, 'rel', type); dom5.setAttribute(link, 'href', dep); dom5.append(head, link); @@ -65,7 +66,7 @@ export class PrefetchTransform extends Transform { if (this.isImportantFile(file)) { // hold on to the file for safe keeping this.fileMap.set(file.path, file); - callback(null, null); + callback(null); } else { callback(null, file); } @@ -81,12 +82,13 @@ export class PrefetchTransform extends Transform { return done(); } this.analyzer.analyzeDependencies.then((depsIndex: DepsIndex) => { - let fragmentToDeps = new Map(depsIndex.fragmentToDeps); + const fragmentToDeps = new Map(depsIndex.fragmentToDeps); if (this.config.entrypoint && this.config.shell) { - let file = this.fileMap.get(this.config.entrypoint); + const file = this.fileMap.get(this.config.entrypoint)!; + console.assert(file != null); // forward shell's dependencies to main to be prefetched - let deps = fragmentToDeps.get(this.config.shell); + const deps = fragmentToDeps.get(this.config.shell); if (deps) { this.pullUpDeps(file, deps, 'prefetch'); } @@ -94,14 +96,15 @@ export class PrefetchTransform extends Transform { this.fileMap.delete(this.config.entrypoint); } - for (let im of this.config.allFragments) { - let file = this.fileMap.get(im); - let deps = fragmentToDeps.get(im); + for (const importUrl of this.config.allFragments) { + const file = this.fileMap.get(importUrl)!; + console.assert(file != null); + const deps = fragmentToDeps.get(importUrl); if (deps) { this.pullUpDeps(file, deps, 'import'); } this.push(file); - this.fileMap.delete(im); + this.fileMap.delete(importUrl); } for (let leftover of this.fileMap.keys()) { diff --git a/src/build/sw-precache.ts b/src/build/sw-precache.ts index 5e144a26..a6a8efe2 100644 --- a/src/build/sw-precache.ts +++ b/src/build/sw-precache.ts @@ -15,10 +15,10 @@ import {SWConfig} from 'polymer-build'; let logger = logging.getLogger('cli.build.sw-precache'); -export function parsePreCacheConfig(configFile: string): Promise { - return new Promise((resolve, _reject) => { +export function parsePreCacheConfig(configFile: string): Promise { + return new Promise((resolve, _reject) => { fs.stat(configFile, (statError) => { - let config: SWConfig; + let config: SWConfig|null = null; // only log if the config file exists at all if (!statError) { try { diff --git a/src/commands/build.ts b/src/commands/build.ts index 0017478a..4ba9dc14 100644 --- a/src/commands/build.ts +++ b/src/commands/build.ts @@ -57,7 +57,7 @@ export class BuildCommand implements Command { js: {}, }; if (options['html.collapseWhitespace']) { - buildOptions.html.collapseWhitespace = true; + buildOptions.html!.collapseWhitespace = true; } logger.debug('building with options', buildOptions); diff --git a/src/commands/lint.ts b/src/commands/lint.ts index bbb55312..ffe048a0 100644 --- a/src/commands/lint.ts +++ b/src/commands/lint.ts @@ -114,6 +114,6 @@ export class LintCommand implements Command { 'config-field': options['config-field'], // NOTE: `no-recursion` has the opposite behavior of `follow-dependencies` 'no-recursion': !followDependencies, - }).then(() => null as void); + }).then(() => undefined); } } diff --git a/src/commands/serve.ts b/src/commands/serve.ts index 50cfffef..d57beb3f 100644 --- a/src/commands/serve.ts +++ b/src/commands/serve.ts @@ -76,7 +76,7 @@ export class ServeCommand implements Command { // Defer dependency loading until this specific command is run const polyserve = require('polyserve'); - let openPath: string; + let openPath: string|undefined; if (config.entrypoint && config.shell) { openPath = config.entrypoint.substring(config.root.length); if (openPath === 'index.html' || openPath === '/index.html') { diff --git a/src/environments/environments.ts b/src/environments/environments.ts index 1aa7a8df..a27d4a70 100644 --- a/src/environments/environments.ts +++ b/src/environments/environments.ts @@ -22,6 +22,7 @@ const environments = new EnvironmentMap(); /** * Builds an environment with the given name. */ -export function buildEnvironment(name: string): Environment { - return environments.has(name) && new (environments.get(name.toLowerCase()))(); +export function buildEnvironment(name: string): Environment|undefined { + const E = environments.get(name.toLowerCase()); + if (E) return new E(); } diff --git a/src/github/github.ts b/src/github/github.ts index 99528a2a..a1900b11 100644 --- a/src/github/github.ts +++ b/src/github/github.ts @@ -24,10 +24,10 @@ export type RequestAPI = request.RequestAPI; private _owner: string; private _repo: string; - static tokenFromFile(filename: string): string { + static tokenFromFile(filename: string): string|null { try { return fs.readFileSync(filename, 'utf8').trim(); } catch (error) { diff --git a/src/init/init.ts b/src/init/init.ts index e8c04d87..9c4e044b 100644 --- a/src/init/init.ts +++ b/src/init/init.ts @@ -74,7 +74,7 @@ function checkIsMinGW(): boolean { */ function getGeneratorDescription(generator: YeomanEnvironment.GeneratorMeta, generatorName: string): GeneratorDescription { const name = getDisplayName(generatorName); - let description: string; + let description: string = ''; if (templateDescriptions.hasOwnProperty(name)) { description = templateDescriptions[name]; @@ -95,7 +95,9 @@ function getGeneratorDescription(generator: YeomanEnvironment.GeneratorMeta, gen } } // If a description exists, format it properly for the command-line - description = (description) ? chalk.dim(' - ' + description) : ''; + if (description.length > 0) { + description = chalk.dim(` - ${description}`); + } return { name: `${name}${description}`, diff --git a/src/polymer-cli.ts b/src/polymer-cli.ts index 00e97f77..b9763dbd 100644 --- a/src/polymer-cli.ts +++ b/src/polymer-cli.ts @@ -114,7 +114,7 @@ export class PolymerCli { globals: ArgDescriptor[] ): ArgDescriptor[] { const mergedArgs = new Map(); - let defaultOption: string = null; + let defaultOption: string|null = null; const addAll = (args: ArgDescriptor[]) => { for (let definition of args) { @@ -154,7 +154,7 @@ export class PolymerCli { } run(): Promise { - const helpCommand = this.commands.get('help'); + const helpCommand = this.commands.get('help')!; const commandNames = Array.from(this.commands.keys()); let parsedArgs: ParsedCommand; logger.debug('running...'); @@ -184,7 +184,8 @@ export class PolymerCli { let commandName = parsedArgs.command; let commandArgs = parsedArgs.argv; - let command = this.commands.get(commandName); + let command = this.commands.get(commandName)!; + console.assert(command != null); logger.debug(`command '${commandName}' found, parsing command args:`, {args: commandArgs}); let commandDefinitions = this.mergeDefinitions(command, globalArguments); diff --git a/tsconfig.json b/tsconfig.json index 838eddef..e750aad0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,6 +6,7 @@ "moduleResolution": "node", "isolatedModules": false, "declaration": false, + "strictNullChecks": true, "noImplicitAny": true, "noUnusedLocals": true, "noUnusedParameters": true, From d06ab467432ecc90ce0e699da2a64c103fcbe009 Mon Sep 17 00:00:00 2001 From: Justin Fagnani Date: Wed, 9 Nov 2016 12:13:00 -0800 Subject: [PATCH 2/2] address review comments --- src/build/prefetch.ts | 10 ++++++---- src/polymer-cli.ts | 19 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/build/prefetch.ts b/src/build/prefetch.ts index d0d442ce..81033bbc 100644 --- a/src/build/prefetch.ts +++ b/src/build/prefetch.ts @@ -85,8 +85,9 @@ export class PrefetchTransform extends Transform { const fragmentToDeps = new Map(depsIndex.fragmentToDeps); if (this.config.entrypoint && this.config.shell) { - const file = this.fileMap.get(this.config.entrypoint)!; - console.assert(file != null); + const file = this.fileMap.get(this.config.entrypoint); + if (file == null) throw new TypeError('file is null'); + // forward shell's dependencies to main to be prefetched const deps = fragmentToDeps.get(this.config.shell); if (deps) { @@ -97,8 +98,9 @@ export class PrefetchTransform extends Transform { } for (const importUrl of this.config.allFragments) { - const file = this.fileMap.get(importUrl)!; - console.assert(file != null); + const file = this.fileMap.get(importUrl); + if (file == null) throw new TypeError('file is null'); + const deps = fragmentToDeps.get(importUrl); if (deps) { this.pullUpDeps(file, deps, 'import'); diff --git a/src/polymer-cli.ts b/src/polymer-cli.ts index b9763dbd..47002840 100644 --- a/src/polymer-cli.ts +++ b/src/polymer-cli.ts @@ -182,21 +182,22 @@ export class PolymerCli { throw error; } - let commandName = parsedArgs.command; - let commandArgs = parsedArgs.argv; - let command = this.commands.get(commandName)!; - console.assert(command != null); + const commandName = parsedArgs.command; + const commandArgs = parsedArgs.argv; + const command = this.commands.get(commandName)!; + if (command == null) throw new TypeError('command is null'); + logger.debug(`command '${commandName}' found, parsing command args:`, {args: commandArgs}); - let commandDefinitions = this.mergeDefinitions(command, globalArguments); - let commandOptionsRaw = commandLineArgs(commandDefinitions, commandArgs); - let commandOptions = parseCLIArgs(commandOptionsRaw); + const commandDefinitions = this.mergeDefinitions(command, globalArguments); + const commandOptionsRaw = commandLineArgs(commandDefinitions, commandArgs); + const commandOptions = parseCLIArgs(commandOptionsRaw); logger.debug(`command options parsed from args:`, commandOptions); - let mergedConfigOptions = Object.assign( + const mergedConfigOptions = Object.assign( {}, this.defaultConfigOptions, commandOptions); - let config = new ProjectConfig(mergedConfigOptions); + const config = new ProjectConfig(mergedConfigOptions); logger.debug(`final project configuration generated:`, config); // Help is a special argument for displaying help for the given command.