enhancement(cli): #1534 replace in-house hashString logic with Node createHash()

[shyok21]

Related Issue

Fixes #1534

Summary of Changes

• Replaced the existing hashString() logic with Node’s built-in createHash() using the MD5 algorithm.
• Updated all test cases referencing hashString() to align with the new implementation

[thescientist13]

Left some comments and this is looking good to start. I think there might be a couple linting failures as @KaiPrince observed in that some variables may now be un-used, so would also be good to double check that yarn lint and yarn format:check are passing locally.

Also, there look to be test cases failing around the matching of hash names, so I would double check locally that yarn test:loaders passes all the test cases (there may be some unrelated test cases failing in actions that will either be solved as part of #1565 or #1457 )

[shyok21]

@thescientist13 @KaiPrince ,
Pushed the changes based on feedback and ran yarn test:loaders to confirm there are no hashing-related issues.

Let me know if any more changes required!

[KaiPrince]

Looks like your CI is failing. It needs to pass before this can be merged. You may need to update from master or rebase. I can review once it's passing.

[thescientist13]

Sorry for the delay in getting back to this, but catching up here and the test failures in GitHub Actions do seem to be consistent with my local testing of the same test cases when running yarn test:loaders.

In particular, it seems that it is CSS files specifically that are failing the tests, in that they aren't being generated? I would consider double checking your local setup, and to start by taking one of the failing test cases and isolating them using xdescribe and applying new Runner(true) to see what might be going on
https://github.com/ProjectEvergreen/greenwood/blob/master/CONTRIBUTING.md#running-tests

Appreciate all the work and collaboration here! 🙇

[shyok21]

Hi @thescientist13 @KaiPrince ,
I have resolved most of the test errors which were coming due to change in hash.

I have also ran yarn test:loaders on master branch and noticed some existing issues that don’t seem to be related to this change.

On last CI run, I found the following issue (related to hash change), which I’ll be fixing:

But the remaining issues appear to be existing.

[thescientist13]

I have also ran yarn test:loaders on master branch and noticed some existing issues that don’t seem to be related to this change.

Hmm, it's hard to say without seeing the whole thing, but that screenshot seems like it could be related to changes here, given the test case that is failing (but perhaps it is #1572 you are seeing)

Aside from that, what operating system are you on and what version of Yarn are you using? We have CI tests for both Linux (Ubuntu) and Windows and multiple versions of NodeJS and none of the open PRs are failing on those test case, but maybe you're system setup is one we've overlooked?

But let me pull this down and play around with it though.

[thescientist13]

OK, I figured out why the GraphQL test cases were still failing, and it was just related to a subtle implementation detail in that the GraphQL plugin also uses hash-utils.js both on the client and the server

• https://github.com/ProjectEvergreen/greenwood/blob/master/packages/plugin-graphql/src/core/common.js#L1
• https://github.com/ProjectEvergreen/greenwood/blob/master/packages/plugin-graphql/src/core/client.js#L1

And now that we are using node:crypto, that will of course break in the browser

So for now, I just changed the GraphQL plugin using the old implementation since I'm not too worried about it, and honestly I'm considering deprecating the GraphQL plugin anyway.

---

I think last thing here is just seeing what we can do about this comment](https://github.com/ProjectEvergreen/greenwood/pull/1564/files#r2333748315), only in so much as I just worry about uniqueness if everything is ending with __.

I'll leave some comments in our ongoing thread there.

[thescientist13]

Thanks for everyone's help on this (and helping me learn a few things along the way!) and so I think this is looking good now.

Will get this included in our next release line, and so will get this merged when the first round of PRs for that is getting queued up!

[thescientist13]

Thanks again and apologies for the delay in merging, but ready to start the new release line, so going to merge this one in!