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

chore(types): add type information #791

Merged
merged 1 commit into from Mar 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/add/index.ts
Expand Up @@ -9,13 +9,13 @@ import modifyConfigHelper from "@webpack-cli/utils/modify-config-helper";
* we're given on a generator
*
*/
const DEFAULT_WEBPACK_CONFIG_FILENAME: string = "webpack.config.js";
const DEFAULT_WEBPACK_CONFIG_FILENAME = "webpack.config.js";

export default function add(...args: string[]): Function {
const filePaths: string[] = args.slice(3);
let configFile: string = DEFAULT_WEBPACK_CONFIG_FILENAME;
const filePaths = args.slice(3);
let configFile = DEFAULT_WEBPACK_CONFIG_FILENAME;
if (filePaths.length) {
configFile = filePaths[0];
configFile = filePaths[0];
}

return modifyConfigHelper("add", defaultGenerator, configFile);
Expand Down
2 changes: 1 addition & 1 deletion packages/generate-loader/index.ts
Expand Up @@ -10,7 +10,7 @@ import { IYeoman } from "./types/Yeoman";

export default function loaderCreator(): void {
const env = yeoman.createEnv();
const generatorName: string = "webpack-loader-generator";
const generatorName = "webpack-loader-generator";

env.registerStub(LoaderGenerator, generatorName);

Expand Down
2 changes: 1 addition & 1 deletion packages/generate-plugin/index.ts
Expand Up @@ -10,7 +10,7 @@ import { IYeoman } from "./types/Yeoman";

export default function pluginCreator(): void {
const env = yeoman.createEnv();
const generatorName: string = "webpack-plugin-generator";
const generatorName = "webpack-plugin-generator";

env.registerStub(PluginGenerator, generatorName);

Expand Down
13 changes: 6 additions & 7 deletions packages/generators/add-generator.ts
Expand Up @@ -11,7 +11,6 @@ import {
AutoComplete,
Confirm,
IInquirerInput,
IInquirerList,
Input,
List,
} from "@webpack-cli/webpack-scaffold";
Expand Down Expand Up @@ -51,7 +50,7 @@ function replaceAt(str: string, index: number, replace: string): string {
* is present
*/
const traverseAndGetProperties = (arr: object[], prop: string): boolean => {
let hasProp: boolean = false;
let hasProp = false;
arr.forEach((p: object): void => {
if (p[prop]) {
hasProp = true;
Expand Down Expand Up @@ -174,7 +173,7 @@ export default class AddGenerator extends Generator {
});
}
}
const temp: string = action;
const temp = action;
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look obvious to me. I would keep the type here

Copy link
Contributor

Choose a reason for hiding this comment

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

action is already a string, so temp inherits its type

if (action === "resolveLoader") {
action = "resolve";
}
Expand Down Expand Up @@ -426,11 +425,11 @@ export default class AddGenerator extends Generator {
.then((p: string) => {
if (p) {
this.dependencies.push(answerToAction.actionAnswer);
const normalizePluginName: string = answerToAction.actionAnswer.replace(
const normalizePluginName = answerToAction.actionAnswer.replace(
"-webpack-plugin",
"Plugin",
);
const pluginName: string = replaceAt(
const pluginName = replaceAt(
normalizePluginName,
0,
normalizePluginName.charAt(0).toUpperCase(),
Expand Down Expand Up @@ -472,7 +471,7 @@ export default class AddGenerator extends Generator {
return;
}
// Either we are adding directly at the property, else we're in a prop.theOne scenario
const actionMessage: string =
const actionMessage =
isDeepProp[1] === "other"
? `What do you want the key on ${
action
Expand Down Expand Up @@ -518,7 +517,7 @@ export default class AddGenerator extends Generator {
);
} else {
// We got the schema prop, we've correctly prompted it, and can add it directly
this.configuration.config.item = action + "." + isDeepProp[1];
this.configuration.config.item = `${action}.${isDeepProp[1]}`;
this.configuration.config.webpackOptions[action] = {
[isDeepProp[1]]: deepPropAns.deepProp,
};
Expand Down
2 changes: 1 addition & 1 deletion packages/generators/addon-generator.ts
Expand Up @@ -46,7 +46,7 @@ export default function addonGenerator(
}

public default() {
const currentDirName: string = path.basename(this.destinationPath());
const currentDirName = path.basename(this.destinationPath());
if (currentDirName !== this.props.name) {
this.log(`
Your project must be inside a folder named ${this.props.name}
Expand Down
4 changes: 2 additions & 2 deletions packages/generators/init-generator.ts
Expand Up @@ -430,7 +430,7 @@ export default class InitGenerator extends Generator {
public installPlugins() {
if (this.isProd) {
this.dependencies = this.dependencies.filter(
(p: string) => p !== "terser-webpack-plugin",
(p: string): boolean => p !== "terser-webpack-plugin",
);
} else {
this.configuration.config.topScope.push(
Expand All @@ -439,7 +439,7 @@ export default class InitGenerator extends Generator {
"\n",
);
}
const packager: string = getPackageManager();
const packager = getPackageManager();
const opts: {
dev?: boolean,
"save-dev"?: boolean,
Expand Down
4 changes: 2 additions & 2 deletions packages/generators/remove-generator.ts
Expand Up @@ -33,8 +33,8 @@ export default class RemoveGenerator extends Generator {
},
};

let configPath: string = path.resolve(process.cwd(), "webpack.config.js");
const webpackConfigExists: boolean = fs.existsSync(configPath);
let configPath = path.resolve(process.cwd(), "webpack.config.js");
const webpackConfigExists = fs.existsSync(configPath);
if (!webpackConfigExists) {
configPath = null;
// end the generator stating webpack config not found or to specify the config
Expand Down
4 changes: 1 addition & 3 deletions packages/generators/utils/validate.ts
Expand Up @@ -7,9 +7,7 @@
* Or a string if the user hasn't written anything
*/
export default function validate(value: string): string | boolean {
const pass: number = value.length;

if (pass) {
if (value.length) {
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/init/index.ts
Expand Up @@ -14,7 +14,7 @@ import npmPackagesExists from "@webpack-cli/utils/npm-packages-exists";
*/

export default function initializeInquirer(...args: string[]): Function | void {
const packages: string[] = args.slice(3);
const packages = args.slice(3);

if (packages.length === 0) {
return modifyConfigHelper("init", defaultGenerator);
Expand Down
10 changes: 5 additions & 5 deletions packages/init/init.ts
Expand Up @@ -42,11 +42,11 @@ export default function runTransform(webpackProperties: IWebpackProperties, acti
.keys(webpackProperties)
.filter((p: string): boolean => p !== "configFile" && p !== "configPath");

const initActionNotDefined: boolean = (action && action !== "init") || false;
const initActionNotDefined = (action && action !== "init") || false;

webpackConfig.forEach((scaffoldPiece: string): Promise<void> => {
const config: IConfiguration = webpackProperties[scaffoldPiece];
const transformations: string[] = mapOptionsToTransform(config);
const transformations = mapOptionsToTransform(config);
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look obvious to me what's the return type of mapOptionsToTransform

Copy link
Member Author

Choose a reason for hiding this comment

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

My inspiration for this and the above one is from the Typescript core repo. Again here the type is obvious if you know the method signature or the variable and we don't need to be verbose.

But well, I agree it is highly subjective...

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually there's no need to assign a type if the function already has a signature (and just one, where it returns always a string).

const ast = j(
initActionNotDefined
? webpackProperties.configFile
Expand All @@ -58,12 +58,12 @@ export default function runTransform(webpackProperties: IWebpackProperties, acti
return astTransform(j, ast, config.webpackOptions[f], transformAction, f);
})
.then((value: string[]): void | PromiseLike <void> => {
let configurationName: string = "webpack.config.js";
let configurationName = "webpack.config.js";
if (config.configName) {
configurationName = "webpack." + config.configName + ".js";
configurationName = `webpack.${config.configName}.js`;
}

const outputPath: string = initActionNotDefined
const outputPath = initActionNotDefined
? webpackProperties.configPath
: path.join(process.cwd(), configurationName);

Expand Down
8 changes: 4 additions & 4 deletions packages/migrate/index.ts
Expand Up @@ -42,14 +42,14 @@ declare var process: {
*/

export default function migrate(...args: string[]): void | Promise<void> {
const filePaths: string[] = args.slice(3);
const filePaths = args.slice(3);
if (!filePaths.length) {
const errMsg: string = "\n ✖ Please specify a path to your webpack config \n ";
const errMsg = "\n ✖ Please specify a path to your webpack config \n ";
console.error(chalk.red(errMsg));
return;
}

const currentConfigPath: string = path.resolve(process.cwd(), filePaths[0]);
const currentConfigPath = path.resolve(process.cwd(), filePaths[0]);
let outputConfigPath: string;

if (!filePaths[1]) {
Expand Down Expand Up @@ -232,7 +232,7 @@ function runMigration(currentConfigPath: string, outputConfigPath: string): Prom
});
})
.catch((err: object): void => {
const errMsg: string = "\n ✖ ︎Migration aborted due to some errors: \n";
const errMsg = "\n ✖ ︎Migration aborted due to some errors: \n";
console.error(chalk.red(errMsg));
console.error(err);
process.exitCode = 1;
Expand Down
6 changes: 3 additions & 3 deletions packages/remove/index.ts
Expand Up @@ -11,10 +11,10 @@ import modifyConfigHelper from "@webpack-cli/utils/modify-config-helper";
*/

export default function remove(...args: string[]): Function {
const DEFAULT_WEBPACK_CONFIG_FILENAME: string = "webpack.config.js";
const DEFAULT_WEBPACK_CONFIG_FILENAME = "webpack.config.js";

const filePaths: string[] = args.slice(3);
let configFile: string = DEFAULT_WEBPACK_CONFIG_FILENAME;
const filePaths = args.slice(3);
let configFile = DEFAULT_WEBPACK_CONFIG_FILENAME;
if (filePaths.length) {
configFile = filePaths[0];
}
Expand Down
2 changes: 1 addition & 1 deletion packages/serve/index.ts
Expand Up @@ -52,7 +52,7 @@ const getRootPathModule = (dep: string): string => path.resolve(process.cwd(), d
*/

function serve() {
const packageJSONPath: string = getRootPathModule("package.json");
const packageJSONPath = getRootPathModule("package.json");
if (!packageJSONPath) {
console.error(
"\n",
Expand Down
6 changes: 3 additions & 3 deletions packages/update/index.ts
Expand Up @@ -11,10 +11,10 @@ import modifyConfigHelper from "@webpack-cli/utils/modify-config-helper";
*/

export default function update(...args: string[]): Function {
const DEFAULT_WEBPACK_CONFIG_FILENAME: string = "webpack.config.js";
const DEFAULT_WEBPACK_CONFIG_FILENAME = "webpack.config.js";

const filePaths: string[] = args.slice(3);
let configFile: string = DEFAULT_WEBPACK_CONFIG_FILENAME;
const filePaths = args.slice(3);
let configFile = DEFAULT_WEBPACK_CONFIG_FILENAME;
if (filePaths.length) {
configFile = filePaths[0];
}
Expand Down
19 changes: 13 additions & 6 deletions packages/utils/ast-utils.ts
Expand Up @@ -8,9 +8,10 @@ import * as validateIdentifier from "./validate-identifier";
* @param {Array} paths - Array of strings containing the traversal path
* @returns {Any} Value at given traversal path
*/

function safeTraverse(obj: INode, paths: string[]): any {
let val: INode = obj;
let idx: number = 0;
let idx = 0;

while (idx < paths.length) {
if (!val) {
Expand All @@ -28,13 +29,19 @@ function safeTraverse(obj: INode, paths: string[]): any {
* @param {Node} path - AST node
* @returns {String|Boolean} type at given path.
*/

function safeTraverseAndGetType(path: INode): string | boolean {
const pathValue: INode = safeTraverse(path, ["value", "value"]);
return pathValue ? pathValue.type : false;
}

// Convert nested MemberExpressions to strings like webpack.optimize.DedupePlugin
function memberExpressionToPathString(path: INode) {
/**
* Convert nested MemberExpressions to strings like webpack.optimize.DedupePlugin
* @param {Node} path - AST node
* @returns {String} member expression string.
*/

function memberExpressionToPathString(path: INode): string {
if (path && path.object) {
return [memberExpressionToPathString(path.object), path.property.name].join(
".",
Expand Down Expand Up @@ -230,7 +237,7 @@ function addOrUpdateConfigObject(

if (propertyExists) {
rootNode.properties
.filter((path: INode) => path.key.name === configProperty)
.filter((path: INode): boolean => path.key.name === configProperty)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed the type here?

.forEach((path: INode) => {
const newProperties = path.value.properties.filter(
(p: INode) => p.key.name !== key,
Expand Down Expand Up @@ -269,7 +276,7 @@ function findAndRemovePluginByName(j: IJSCodeshift, node: INode, pluginName: str
let rootPath: INode;

findPluginsByName(j, node, [pluginName])
.filter((path: INode) => safeTraverse(path, ["parent", "value"]))
.filter((path: INode): boolean => safeTraverse(path, ["parent", "value"]))
.forEach((path: INode) => {
rootPath = safeTraverse(path, ["parent", "parent", "parent", "value"]);
const arrayPath: INode = path.parent.value;
Expand Down Expand Up @@ -324,7 +331,7 @@ function createOrUpdatePluginByName(j: IJSCodeshift, rootNodePath: INode, plugin
// Search for same keys in the existing object
const existingProps = j(currentProps)
.find(j.Identifier)
.filter((p: INode) => opt.key.value === p.value.name);
.filter((p: INode): boolean => opt.key.value === p.value.name);

if (existingProps.size()) {
// Replacing values for the same key
Expand Down
8 changes: 4 additions & 4 deletions packages/utils/copy-utils.ts
Expand Up @@ -20,9 +20,9 @@ export const generatorCopy = (
generator,
templateDir: string,
): (filePath: string) => void => (filePath: string): void => {
const sourceParts: string[] = templateDir.split(path.delimiter);
const sourceParts = templateDir.split(path.delimiter);
sourceParts.push.apply(sourceParts, filePath.split("/"));
const targetParts: string[] = path.dirname(filePath).split("/");
const targetParts = path.dirname(filePath).split("/");
targetParts.push(path.basename(filePath, ".tpl"));

generator.fs.copy(
Expand All @@ -47,9 +47,9 @@ export const generatorCopyTpl = (
templateDir: string,
templateData: object,
): (filePath: string) => void => (filePath: string): void => {
const sourceParts: string[] = templateDir.split(path.delimiter);
const sourceParts = templateDir.split(path.delimiter);
sourceParts.push.apply(sourceParts, filePath.split("/"));
const targetParts: string[] = path.dirname(filePath).split("/");
const targetParts = path.dirname(filePath).split("/");
targetParts.push(path.basename(filePath, ".tpl").slice(1));

generator.fs.copyTpl(
Expand Down