Code review for YG TECHNOLOGY INC #1
Conversation
Analysis Details111 Issues
Coverage and Duplications
Project ID: automazeio_diffy-worker-review_27ac4111-009b-4b36-83d0-aa16b1dbde3c |
There was a problem hiding this comment.
Move project to the TypeScript,
Remove unused dependencies and dependencies with doubling of functionality,
Use built in Node JS features,
Update JSDoc ,
Give understandable names to variables,
Replace let and var with const where necessary and also replace all var with let where it is not possible to use const,
Add formatter and linter and configs for them (we recommend using the 2 in 1 library “biome.js”),
Remove useless “Promise.resolve()”,
Prefer using “for...of” instead of “forEach”,
Reject promises when an error occurs and also add error handling, template literals are Preferred over string concatenation
Do not access Object.prototype method 'hasOwnProperty' from target object
Most checks in if blocks can be simplified to an optional chain.
Prefer using arrow functions
| // Example to run | ||
| // node diffy-screenshots.js --url=https://diffy.website | ||
|
|
||
| const debug = false |
There was a problem hiding this comment.
Can be moved to the env
|
|
||
| const debug = false | ||
|
|
||
| require('dotenv').config(); |
There was a problem hiding this comment.
Can be replaced with built in Node JS env
| const jobs = new Jobs(logger) | ||
|
|
||
| const { Api } = require('./lib/api.js') | ||
| let api |
There was a problem hiding this comment.
The variable declaration must be changed to const also it can be moved the initialization location.
| await end() | ||
| }); | ||
|
|
||
| (async () => { |
There was a problem hiding this comment.
execSync blocks the main thread, so the execution does not take place in parallel, but sequentially, which is directly proportional to the number of tasks.
execSync can be replaced with execAsync and in general you can abandon the for cycle in favor of map and process all promises through Promise.all
| // file-content -- if we pass job file as json as parameter | ||
| // output-filepath -- path to a file to save the results in json format. Used by wrapper. | ||
|
|
||
| require('dotenv').config(); |
There was a problem hiding this comment.
Can be replaced with built in Node JS env
There was a problem hiding this comment.
Do not access Object.prototype method 'hasOwnProperty' from target object
Most checks in if blocks can be simplified to an optional chainDo not access Object.prototype method
In many cases, the ternary operator can be replaced by "?? " operator that will reduce a large part of the verification code
Replace let and var with const where necessary and also replace all var with let where it is not possible to use const
Prefer to use the more modern "@aws-sdk/client-sqs" package, as "aws sdk v2" will soon lose support
There was a problem hiding this comment.
“imagemagick” can be replaced using exist library with more extended functionality “sharp”
Prefer to use arrow functions
Using "sharp", you can refuse callbacks and additional wrapping of promises
There was a problem hiding this comment.
“bluebird” can be replaced with native promises
Prefer to use the more modern "@aws-sdk/client-s3" package, as "aws sdk v2" will soon lose support
Replace let and var with const where necessary and also replace all var with let where it is not possible to use const
Template literals are preferred over string concatenation
You can get rid of the additional wrapping of promises by using more modern libraries such as "fs/promises" and "@aws-sdk/client-s3."
Prefer to use arrow functions
| .then((fileData) => { | ||
| params.Body = fileData; | ||
| return Promise | ||
| .promisify(s3.putObject, { |
There was a problem hiding this comment.
There is a better option for simultaneous recording and sending of a file, namely createReadStream
There was a problem hiding this comment.
aws-sdk: It is recommended to upgrade to version 3, as it offers improved performance and a reduced library size
bluebird: built-in promises are quite performant and feature-rich.
dotenv: built-in simillar feature
fs-extra: built-in fs has the same functionality
gm and imagemagick: the same functionality already present in sharp
iltorb: deprecated and can be replaced with any modern bundler
minimist and yargs: both libraries provide similar functionality, since you are already using minimist, you can remove yargs
request: deprecated , and no longer maintained. It is recommended to switch to modern alternatives such as axios or fetch.
No description provided.