Repository navigation
buildVersionHash is not doing what is suppose to do #31
Description
Hi, buildVersionHash has a comment (and historically do this):
Walks the directory tree, finding files, generating a version hash
But from v.3.0.1, the code responsible for the hash creation (cachedMakeHash) is called from getVersionedPath. So, the comment is outdated but most important, the whole concept of
When you initialize this module, it crawls your public folder synchronously at startup, and pre-determines all the MD5 hashes for your static files. This slows down application startup slightly, but it keeps the runtime performance at its peak.
Now, the hash is generated only if the resource is called and make slow the first call of it.
I think that, or you change your code to match the documentation (and how historically works) or you change the description/comment.
Let me know what you think.
Activity
XhmikosR commentedon Apr 9, 2019
You mean v3.1.0? You can always submit a PR :)
dariomac-vix commentedon Apr 9, 2019
Yes... sorry, v3.1.0... and yes... I can submit a PR.
I'm only asking because it was a big change and maybe it has some reason behind that I'm not aware. I'll test some changes locally and submit a PR asap.
XhmikosR commentedon Apr 9, 2019
It was just a leftover I believe. Because the
buildVersionHashdoes do what it says, but now it depends oncachedMakeHashso the comment(s) might need to be adapted..dariomac-vix commentedon Apr 9, 2019
Mmm but looking at the code I can't figure it out how it does what it says, because in any case call
cachedMakeHashand that's is the only place that calledcrypto.createHashXhmikosR commentedon Apr 9, 2019
@Rush: do you have any comments since you were the author of the patch?
Rush commentedon Apr 9, 2019
Comment is no longer valid. We shouldn't by default warm up the cache. (Perhaps optionally) Why would we slowdown the server upon every reload? Think of nodemon or node-dev in development.
Rush commentedon Apr 9, 2019
See my previous PR for context #23
dariomac-vix commentedon Apr 9, 2019
I think it can be optional as you say... With your context message I understand why you did that. Also, not only the comment is no longer valid, but parts of the readme.md too (as the one I quoted early).
Tonight I'll update my PR trying to include changes in the readme.md