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

Update dependencies to latest available versions #213

Merged
merged 3 commits into from
Sep 30, 2022

Conversation

BaCaRoZzo
Copy link
Collaborator

@BaCaRoZzo BaCaRoZzo commented Sep 16, 2022

Fixes #195

Used node 14.20.0 which is the latest version of the current LTS. I'm personally in love with yarn but ofc I've used npm to do the update. But I would suggest to move to yarn since it's way faster.

Aside from the update, the PR drops chalk due to its limited usage in favour of using the colors directly. Maybe it's not pretty as before but it doesn't really make sense to introduce a library for a couple of simple usages.

istanbul should be replaced by nyc? I didn't delve into that option but I guess it should be taken in account. I would also consider to drop moment since it is used sparingly and its a huge dep. But I'll leave that to you as well since there are too many alternatives and too many opinions regarding them. 😄

EDIT
Actually also find is unmaintained and very old. 😱 I feel like that can be substituted by a function, something along the line of:

const fsPromises = require('fs').promises
const files = await fsPromises.readdir(folder, {withFileTypes: true})
// simple `filter()` call?

EDIT 2
Dropped both moment and istanbul in favor of luxon and nyc. While the latter was an obvious move, luxon choice came because it's the only one with a sound implementation of duration formatting. Both dayjs and date-fns are bugged/not feature complete in that regard.
It should be noted that moment format function was resetting on a 24h basis. Since some tests were interpreting ns times as ms (durationInMS=true) that would have resulted in huge duration intervals but the behaviour of moment was successfully hiding that. luxon made the problem apparent and thus I've updated those tests to have "saner" duration intervals.

Overall happy about these changes, dropping moment will reduce greatly the bundle since luxon is almost half the size!

Also the other minor deps can be probably dropped, as mentioned for find, but I think in those cases it really tumbles down to personal taste. I would not keep around a dep for a simple task or one that is unmaintained like find. Although, for this iteration, I think we have made enough changes. 🙂

Drop also "chalk" due to its limited usage in favour of using the colors directly.
It appears some tests has durations set in ns but were used with time assumed in ms (durationInMS=true). Since moment.js format function resets on a 24h basis those huge durations went unnoticed.
Luxon correctly handles durations over 24h, making it apparent that some tests had an insane duration of almost 6 months.
Hence, as part of the migration to Luxon, tests were updated to have saner durations.
@BaCaRoZzo
Copy link
Collaborator Author

BaCaRoZzo commented Sep 18, 2022

@wswebcreation hope you don't mind for me to have stolen the issue from your hand. It's somewhat relaxing at times to take care of the deps bump. 🥲

Copy link
Collaborator

@wswebcreation wswebcreation left a comment

Choose a reason for hiding this comment

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

Hi @BaCaRoZzo

I'm so sorry for responding so late, I was traveling a lot for work and didn't had the time to look at it.

Thanks for this, it really looks great!

@wswebcreation wswebcreation merged commit d3f6596 into WasiqB:main Sep 30, 2022
@wswebcreation
Copy link
Collaborator

And regarding npm vs yarn. I totally agree with you, but npm is still the "standard" for a lot of users so would love to keep it this way

@BaCaRoZzo
Copy link
Collaborator Author

I'm so sorry for responding so late

Oh no worries. It was good because it finally triggered a choice on my side for luxon.

And regarding npm vs yarn. I totally agree with you, but npm is still the "standard" for a lot of users so would love to keep it this way

Ofc! Your project, your rules @wswebcreation. I was just forced a couple of years ago to use yarn and since then, when I get back to npm, I feel indeed a bit of slowness. But that's it.
Looking forward to using the new release. Thanks a lot and, since it's Friday, enjoy the week-end!! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a reason to use such an old version of the UUID dependency?
2 participants