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

Fixes #147 (rev replaced with hash) #152

Merged
merged 3 commits into from
Mar 12, 2017
Merged

Conversation

EugeneZ
Copy link
Contributor

@EugeneZ EugeneZ commented Mar 11, 2017

  • Replaced md5 with crc because crc will produce a short 32-bit digest (md5 is 128-bit). Also faster/simpler, though that doesn't really matter.
  • Using left-pad and Buffer to encode the 32-bit crc digest as a base64-encoded string. The left-pad is used because crc returns a Number which sometimes results in a "short" string (eg 12345667 instead of 01234567), and Buffer with hex decoding expects a series of character pairs. We strip the equal signs base64 uses for padding (those are the characters we saved). Ideally, it'd be nice to use base65536 instead of base64, but I'm worried we'll run into some weird Chrome bugs or something. I'd like to try this anyway in the future, it would reduce the hash length to two characters.
  • Removed the revision code because it was being performed during the hot reloading and we need to add the hash during the initial file processing
  • I realized how I messed up (@pizza2code) the file names in my previous PR, I was using Browserify's filename and blowing away the filename in the sourcemap. Now I'm pulling the filename from the sourcemap and appending the hash to that.

I'd like to call the ?version=hash param as ?v=hash instead to make it shorter but I promised @pizza2code I'd make it explicit. It's ultimately up to you, change it to whatever you want.

@milankinen
Copy link
Owner

Looks good! Thank you very much!

@milankinen milankinen merged commit b6fbe69 into milankinen:master Mar 12, 2017
@milankinen
Copy link
Owner

...and released in 3.3.0!

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.

2 participants