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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
|
@@ -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( | ||
".", | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -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; | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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, sotemp
inherits its type