Conversation
|
|
||
| ```javascript | ||
| const licenseFile = require('nodejs-license-file'); | ||
| const licenseFile = require("nodejs-license-file"); |
There was a problem hiding this comment.
Why change the string quotes convention?
There was a problem hiding this comment.
This project does not have an .eslintrc file so you shouldn't be applying your personal preferences to it. It's better to maintain the existing conventions.
| firstName: "Name", | ||
| lastName: "Last Name", | ||
| email: "some@email.com", | ||
| expirationDate: "2025-11-15" |
There was a problem hiding this comment.
Why change the date format? Used to be separated by /, and now separated by -.
There was a problem hiding this comment.
dayjs handles date with this specific format by default, plus it's less confusing
I didnt know in the first place if it was DD/MM/YYYY or MM/DD/YYYY
| }, | ||
| "homepage": "https://github.com/bushev/nodejs-license-file", | ||
| "dependencies": { | ||
| "dayjs": "1.8.17", |
There was a problem hiding this comment.
It's trivial to compare dates in JavaScript, so you don't need a dependency for that.
There was a problem hiding this comment.
It's easier to manipulate dates with dayjs, which is the lightest date library I've found
There was a problem hiding this comment.
While that's true, the manipulation that you perform is rather trivial even without a library.
In lib/index.js:239 you're just comparing two dates. I'll comment there directly.
| @@ -0,0 +1,218 @@ | |||
| # THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. | |||
There was a problem hiding this comment.
The project doesn't currently have a yarn.lock, so please don't commit one.
|
That's a useful feature that we're currently doing in application code instead of via the dependency. |
| const expirationDate = data.expirationDate; | ||
| const isBefore = dayjs().isBefore(dayjs(expirationDate)); | ||
| const isSame = dayjs().isSame(dayjs(expirationDate)) | ||
| return isBefore || isSame; |
There was a problem hiding this comment.
What I see you're doing is checking if now <= expirationDate. You don't need a library for that.
I believe you can compare two plain JavaScript date objects with the <= operator directly.
If that's hard to grasp, you can also convert them to a UNIX timestamp (a number) and compare those.
https://stackoverflow.com/questions/492994/compare-two-dates-with-javascript
You can get a date object of the current time via the date constructor without parameters new Date(). Alternatively, you can get the UNIX timestamp by calling Date.now().
So, it would be something like:
return (new Date()) <= new Date(expirationDate)
or
return Date.now() <= (new Date(expirationDate)).getTime()
2 participants
If there is no expirationDate specified in the license then it returns true, otherwise it checks if today is before or same day as expirationDate