Skip to content

Commit

Permalink
Enable TypeScript strictNullChecks (#2755)
Browse files Browse the repository at this point in the history 
* Enable strictNullChecks

There were 400 errors to fix and most of the mwere fixed by adding explicit type
assertions at every error site. It would be a good idea to go back and
clean some of these up later.

There are a few places that I changed the logic or the types, which I've
tried to point out in the pull request comments.

* Revert logic changes to fix tests

* Fix remaining missing types

* Enforce consistent type-casting style

* Explicitly list strict options we are still missing
  • Loading branch information
@edsrzf @lukastaegert
edsrzf authored and lukastaegert committed May 17, 2019
1 parent a4fbc53 commit 65b6aef
Show file tree
Hide file tree
Showing 86 changed files with 672 additions and 556 deletions.
6 changes: 3 additions & 3 deletions bin/src/logging.ts
View file
Expand Up @@ -9,8 +9,8 @@ export function handleError(err: RollupError, recover = false) {
let description = err.message || err;
if (err.name) description = `${err.name}: ${description}`;
const message =
((<{ plugin?: string }>err).plugin
? `(${(<{ plugin?: string }>err).plugin} plugin) ${description}`
((err as { plugin?: string }).plugin
? `(${(err as { plugin?: string }).plugin} plugin) ${description}`
: description) || err;

stderr(tc.bold.red(`[!] ${tc.bold(message.toString())}`));
Expand All @@ -20,7 +20,7 @@ export function handleError(err: RollupError, recover = false) {
}

if (err.loc) {
stderr(`${relativeId(err.loc.file || err.id)} (${err.loc.line}:${err.loc.column})`);
stderr(`${relativeId((err.loc.file || err.id) as string)} (${err.loc.line}:${err.loc.column})`);
} else if (err.id) {
stderr(relativeId(err.id));
}
Expand Down
47 changes: 25 additions & 22 deletions bin/src/run/batchWarnings.ts
View file
Expand Up @@ -23,13 +23,13 @@ export default function batchWarnings() {
warning = { code: 'UNKNOWN', message: warning };
}

if (warning.code in immediateHandlers) {
immediateHandlers[warning.code](warning);
if ((warning.code as string) in immediateHandlers) {
immediateHandlers[warning.code as string](warning);
return;
}

if (!allWarnings.has(warning.code)) allWarnings.set(warning.code, []);
allWarnings.get(warning.code).push(warning);
if (!allWarnings.has(warning.code as string)) allWarnings.set(warning.code as string, []);
(allWarnings.get(warning.code as string) as RollupWarning[]).push(warning);

count += 1;
},
Expand All @@ -44,17 +44,20 @@ export default function batchWarnings() {

if (deferredHandlers[a]) return -1;
if (deferredHandlers[b]) return 1;
return allWarnings.get(b).length - allWarnings.get(a).length;
return (
(allWarnings.get(b) as RollupWarning[]).length -
(allWarnings.get(a) as RollupWarning[]).length
);
});

codes.forEach(code => {
const handler = deferredHandlers[code];
const warnings = allWarnings.get(code);

if (handler) {
handler.fn(warnings);
handler.fn(warnings as RollupWarning[]);
} else {
warnings.forEach(warning => {
(warnings as RollupWarning[]).forEach(warning => {
title(warning.message);

if (warning.url) info(warning.url);
Expand Down Expand Up @@ -91,12 +94,12 @@ const immediateHandlers: {
title(`Missing shims for Node.js built-ins`);

const detail =
warning.modules.length === 1
? `'${warning.modules[0]}'`
: `${warning.modules
(warning.modules as string[]).length === 1
? `'${(warning.modules as string[])[0]}'`
: `${(warning.modules as string[])
.slice(0, -1)
.map((name: string) => `'${name}'`)
.join(', ')} and '${warning.modules.slice(-1)}'`;
.join(', ')} and '${(warning.modules as string[]).slice(-1)}'`;
stderr(
`Creating a browser bundle that depends on ${detail}. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins`
);
Expand Down Expand Up @@ -156,9 +159,9 @@ const deferredHandlers: {
info('https://rollupjs.org/guide/en#error-name-is-not-exported-by-module-');

warnings.forEach(warning => {
stderr(tc.bold(warning.importer));
stderr(tc.bold(warning.importer as string));
stderr(`${warning.missing} is not exported by ${warning.exporter}`);
stderr(tc.gray(warning.frame));
stderr(tc.gray(warning.frame as string));
});
},
priority: 1
Expand Down Expand Up @@ -195,10 +198,10 @@ const deferredHandlers: {
title(`Conflicting re-exports`);
warnings.forEach(warning => {
stderr(
`${tc.bold(relativeId(warning.reexporter))} re-exports '${
`${tc.bold(relativeId(warning.reexporter as string))} re-exports '${
warning.name
}' from both ${relativeId(warning.sources[0])} and ${relativeId(
warning.sources[1]
}' from both ${relativeId((warning.sources as string[])[0])} and ${relativeId(
(warning.sources as string[])[1]
)} (will be ignored)`
);
});
Expand All @@ -213,7 +216,7 @@ const deferredHandlers: {
`Use output.globals to specify browser global variable names corresponding to external modules`
);
warnings.forEach(warning => {
stderr(`${tc.bold(warning.source)} (guessing '${warning.guess}')`);
stderr(`${tc.bold(warning.source as string)} (guessing '${warning.guess}')`);
});
},
priority: 1
Expand Down Expand Up @@ -252,7 +255,7 @@ const deferredHandlers: {
nestedByMessage.forEach(({ key: message, items }) => {
title(`${plugin} plugin: ${message}`);
items.forEach(warning => {
if (warning.url !== lastUrl) info((lastUrl = warning.url));
if (warning.url !== lastUrl) info((lastUrl = warning.url as string));

if (warning.id) {
let loc = relativeId(warning.id);
Expand Down Expand Up @@ -283,17 +286,17 @@ function nest<T>(array: T[], prop: string) {
const lookup = new Map<string, { items: T[]; key: string }>();

array.forEach(item => {
const key = (<any>item)[prop];
const key = (item as any)[prop];
if (!lookup.has(key)) {
lookup.set(key, {
items: [],
key
});

nested.push(lookup.get(key));
nested.push(lookup.get(key) as { items: T[]; key: string });
}

lookup.get(key).items.push(item);
(lookup.get(key) as { items: T[]; key: string }).items.push(item);
});

return nested;
Expand All @@ -305,7 +308,7 @@ function showTruncatedWarnings(warnings: RollupWarning[]) {
const sliced = nestedByModule.length > 5 ? nestedByModule.slice(0, 3) : nestedByModule;
sliced.forEach(({ key: id, items }) => {
stderr(tc.bold(relativeId(id)));
stderr(tc.gray(items[0].frame));
stderr(tc.gray(items[0].frame as string));

if (items.length > 1) {
stderr(`...and ${items.length - 1} other ${items.length > 2 ? 'occurrences' : 'occurrence'}`);
Expand Down
22 changes: 13 additions & 9 deletions bin/src/run/build.ts
View file
Expand Up @@ -6,7 +6,8 @@ import {
OutputAsset,
OutputChunk,
OutputOptions,
RollupBuild
RollupBuild,
SourceMap
} from '../../../src/rollup/types';
import relativeId from '../../../src/utils/relativeId';
import { handleError, stderr } from '../logging';
Expand All @@ -23,16 +24,18 @@ export default function build(
const useStdout = !outputOptions[0].file && !outputOptions[0].dir;

const start = Date.now();
const files = useStdout ? ['stdout'] : outputOptions.map(t => relativeId(t.file || t.dir));
const files = useStdout
? ['stdout']
: outputOptions.map(t => relativeId(t.file || (t.dir as string)));
if (!silent) {
let inputFiles: string;
let inputFiles: string = undefined as any;
if (typeof inputOptions.input === 'string') {
inputFiles = inputOptions.input;
} else if (inputOptions.input instanceof Array) {
inputFiles = inputOptions.input.join(', ');
} else if (typeof inputOptions.input === 'object' && inputOptions.input !== null) {
inputFiles = Object.keys(inputOptions.input)
.map(name => (<Record<string, string>>inputOptions.input)[name])
.map(name => (inputOptions.input as Record<string, string>)[name])
.join(', ');
}
stderr(tc.cyan(`\n${tc.bold(inputFiles)} → ${tc.bold(files.join(', '))}...`));
Expand All @@ -53,12 +56,13 @@ export default function build(
return bundle.generate(output).then(({ output: outputs }) => {
for (const file of outputs) {
let source: string | Buffer;
if ((<OutputAsset>file).isAsset) {
source = (<OutputAsset>file).source;
if ((file as OutputAsset).isAsset) {
source = (file as OutputAsset).source;
} else {
source = (<OutputChunk>file).code;
source = (file as OutputChunk).code;
if (output.sourcemap === 'inline') {
source += `\n//# ${SOURCEMAPPING_URL}=${(<OutputChunk>file).map.toUrl()}\n`;
source += `\n//# ${SOURCEMAPPING_URL}=${((file as OutputChunk)
.map as SourceMap).toUrl()}\n`;
}
}
if (outputs.length > 1)
Expand All @@ -68,7 +72,7 @@ export default function build(
});
}

return Promise.all(outputOptions.map(output => <Promise<any>>bundle.write(output))).then(
return Promise.all(outputOptions.map(output => bundle.write(output) as Promise<any>)).then(
() => bundle
);
})
Expand Down
7 changes: 4 additions & 3 deletions bin/src/run/index.ts
View file
@@ -1,6 +1,6 @@
import { realpathSync } from 'fs';
import relative from 'require-relative';
import { InputOptions } from '../../../src/rollup/types';
import { InputOptions, WarningHandler } from '../../../src/rollup/types';
import mergeOptions from '../../../src/utils/mergeOptions';
import { getAliasName } from '../../../src/utils/relativeId';
import { handleError } from '../logging';
Expand Down Expand Up @@ -89,7 +89,7 @@ export default function runRollup(command: any) {
.then(configs => execute(configFile, configs, command))
.catch(handleError);
} else {
return execute(configFile, <any>[{ input: null }], command);
return execute(configFile, [{ input: null }] as any, command);
}
}

Expand All @@ -107,7 +107,8 @@ function execute(configFile: string, configs: InputOptions[], command: any) {
defaultOnWarnHandler: warnings.add
});

if (optionError) inputOptions.onwarn({ code: 'UNKNOWN_OPTION', message: optionError });
if (optionError)
(inputOptions.onwarn as WarningHandler)({ code: 'UNKNOWN_OPTION', message: optionError });
return build(inputOptions, outputOptions, warnings, command.silent);
});
}
Expand Down
32 changes: 19 additions & 13 deletions bin/src/run/watch.ts
View file
Expand Up @@ -8,7 +8,9 @@ import {
InputOption,
RollupBuild,
RollupError,
RollupWatchOptions
RollupWatchOptions,
WarningHandler,
WatcherOptions
} from '../../../src/rollup/types';
import mergeOptions from '../../../src/utils/mergeOptions';
import relativeId from '../../../src/utils/relativeId';
Expand Down Expand Up @@ -45,7 +47,9 @@ export default function watch(
let processConfigsErr: any;
const initialConfigs = processConfigs(configs);

const clearScreen = initialConfigs.every(config => config.watch.clearScreen !== false);
const clearScreen = initialConfigs.every(
config => (config.watch as WatcherOptions).clearScreen !== false
);

const screen = alternateScreen(isTTY && clearScreen);
screen.open();
Expand All @@ -69,14 +73,14 @@ export default function watch(
if (!result.watch) result.watch = {};

if (merged.optionError)
merged.inputOptions.onwarn({
(merged.inputOptions.onwarn as WarningHandler)({
code: 'UNKNOWN_OPTION',
message: merged.optionError
});

if (
(<RollupWatchOptions>merged.inputOptions).watch &&
(<RollupWatchOptions>merged.inputOptions).watch.clearScreen === false
(merged.inputOptions as RollupWatchOptions).watch &&
((merged.inputOptions as RollupWatchOptions).watch as WatcherOptions).clearScreen === false
) {
processConfigsErr = stderr;
}
Expand All @@ -94,13 +98,13 @@ export default function watch(
switch (event.code) {
case 'FATAL':
screen.close();
handleError(event.error, true);
handleError(event.error as RollupError, true);
process.exit(1);
break;

case 'ERROR':
warnings.flush();
handleError(event.error, true);
handleError(event.error as RollupError, true);
break;

case 'START':
Expand All @@ -115,13 +119,15 @@ export default function watch(
if (typeof input !== 'string') {
input = Array.isArray(input)
? input.join(', ')
: Object.keys(input)
.map(key => (<Record<string, string>>input)[key])
: Object.keys(input as Record<string, string>)
.map(key => (input as Record<string, string>)[key])
.join(', ');
}
stderr(
tc.cyan(
`bundles ${tc.bold(input)} → ${tc.bold(event.output.map(relativeId).join(', '))}...`
`bundles ${tc.bold(input)} → ${tc.bold(
(event.output as string[]).map(relativeId).join(', ')
)}...`
)
);
}
Expand All @@ -132,9 +138,9 @@ export default function watch(
if (!silent)
stderr(
tc.green(
`created ${tc.bold(event.output.map(relativeId).join(', '))} in ${tc.bold(
ms(event.duration)
)}`
`created ${tc.bold(
(event.output as string[]).map(relativeId).join(', ')
)} in ${tc.bold(ms(event.duration as number))}`
)
);
if (event.result && event.result.getTimings) {
Expand Down
4 changes: 2 additions & 2 deletions browser/path.ts
View file
Expand Up @@ -28,7 +28,7 @@ export function dirname(path: string) {
}

export function extname(path: string) {
const match = /\.[^.]+$/.exec(basename(path));
const match = /\.[^.]+$/.exec(basename(path) as string);
if (!match) return '';
return match[0];
}
Expand Down Expand Up @@ -57,7 +57,7 @@ export function relative(from: string, to: string) {
}

export function resolve(...paths: string[]) {
let resolvedParts = paths.shift().split(/[/\\]/);
let resolvedParts = (paths.shift() as string).split(/[/\\]/);

paths.forEach(path => {
if (isAbsolute(path)) {
Expand Down

0 comments on commit 65b6aef

Please sign in to comment.