30-seconds-of-code: Why hashNode return sha256 hex in next tick?

[cncolder]

Description

Is there some security reasons?

[skatcat31]

I'm not exactly sure. I'm pretty certain you could easily just do

const hashNode = val => Promise.resolve(crypto
  .createHash('sha256')
  .update(val)
  .digest('hex')
)

[skatcat31]

@Chalarangelo is there a reason that's done?

[Chalarangelo]

That's one of the oldest snippets, I think we kind of adapted this from some StackOverflow answer, so that's why it looks that way. If the way it works and its use cases are not altered, we should update it to this new version.

[cncolder]

IMO. Promise is unnecessary here also.

1. It cannot avoid process blocking.
2. It cannot provide result memo.

[Chalarangelo]

Two reasons why we are using a promise here:

1. Hashing is quite expensive, so a promise is preferred to make sure the thread doesn't wait for a long time.
2. The footprint matches the browser footprint, which is somewhat nice.

I am pretty sure we should stick to this returning a promise, just clean up the code a bit. Plus, it's gonna be a breaking change and this is quite a popular snippet in my experience.

[skatcat31]

@Chalarangelo this is a blocking stream, wrapping it in a promise merely delays when you can use it, which would mean that it actually works as finish hash, update hash, next available loop return hash.

Although I think this answers why we put it on next tick. By putting it on next tick we offload it until the event loop is free, which since these algorithms can be heavy would in fact be preferred than randomly running a blocking hash out of nowhere.

@cncolder Looks like we remembered why it's done next tick. It's to prevent a long blocking operation and use the event loop to allow for other operations to finish being queued instead of delayed in case of a large hashing operation(everything that gets added after will be delayed though)

[skatcat31]

My suggestion is that hashNode is updated to explain why it uses the setTimeout so as to put it onto the event loop and not block

[Chalarangelo]

@skatcat31 My thoughts exactly. Could you PR an update for the explanation, just so we remember why this is done like this next time? 😛

[skatcat31]

Sure. When I get home tomorrow I'll issue the pr

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for any follow-up tasks.