No unused variables #449
No unused variables #449
Conversation
| @@ -22,7 +22,7 @@ module.exports = function(dependencies, components){ | |||
| delete require.cache[pathToModule]; | |||
| } | |||
|
|
|||
| var required = require(pathToModule); | |||
| require(pathToModule); | |||
There was a problem hiding this comment.
I do not understand what this piece of code does 😊
There was a problem hiding this comment.
The easiest way to know if you can require something is to wrap a require into a try/catch. Given the only value of this is to ensure this doesn't throw (this module gets a list of required dependencies for a module that are not installed), no need to assign the value to a "required" variable (which, in fact, the linter finds as unused).
The reason for the previous lines is that require has an internal cache and if you required something before and then installed it on the flight (by programmatically using npm as we do as result of this module's execution) it remembers the dep doesn't exist, so here comes the need of invalidating this cache to be sure a lookup for that dep is performed again.
Consider that another way to achieve this is to verify the existence of __dirname/node_modules/dependency but this is crazy complicated because of how different versions of npm and yarn save deps in node_modules folder. Also require has a lot of logic to try to guess where your dependency is and it's just convenient to rely on it for that.
| @@ -1,7 +1,7 @@ | |||
| 'use strict'; | |||
|
|
|||
| var async = require('async'); | |||
| var colors = require('colors'); | |||
| var colors = require('colors/safe'); | |||
There was a problem hiding this comment.
colors exposes the classic mode, which just overrides global strings by allowing to do "string".red or the "safe" mode colors.red("string"). We use the safe mode almost everywhere but we missed this before..
| @@ -17,7 +17,7 @@ | |||
| "plusplus" : false, // true: Prohibit use of `++` & `--` | |||
| "quotmark" : "single", // Quotation mark consistency: | |||
| "undef" : true, // true: Require all non-global variables to be declared (prevents global leaks) | |||
| "unused" : false, // true: Require all defined variables be used | |||
| "unused" : true, // true: Require all defined variables be used | |||
|
@matteofigus thanks, shall I merge this? |
Removed some 💩 by setting no unused var setting in jshint and then correcting all the stuff.