rev=<revision_number> added to hot-reloaded module path results in Chrome not pausing on breakpoints

[itsravenous]

(See #103)

This means Chrome sees the script as a different file so any breakpoints don't carry over.

[itsravenous]

As a quick workaround tried hacking reloading.js#166 to always set rev to 1, but quickly discovered that that means Chrome doesn't reload the file contents :)

[cguinnup]

This is a workaround for Chrome failing to hot-reload sourcemaps, which you noticed when trying to fix the issue. Unfortunately, there appears to be no perfect solution.

Would certainly help to star Chromium's open bug: https://bugs.chromium.org/p/chromium/issues/detail?id=438251

[milankinen]

Hi @itsravenous and thanks reporting this! Like you noticed and @pizza2code confirmed, the added revision is needed to display the reloaded sources. Unfortunately this also discards the breakpoints from the previous source. 😥

Currently I don't have any solutions for this. If anyone knows/has any (even partial) solutions, all help is welcome!

[EugeneZ]

Well, the hash-as-pathname approach I used was considerably less annoying in this regard, because two identical files would have the same hash and thus the same filename. Perhaps we should go back to that but put the hash at the end so that's its easier to read? And use a shorter hash value, too? path/to/my/file.js/83af0128cc or something.

[cguinnup]

AFAIK, we only append or increment a version number when the hash has changed. Correct me if I'm wrong.

[EugeneZ]

@pizza2code Yes, but this is done once a server is up and running. So if you load a page, you get the un-versioned file (good). Then you set a breakpoint. Then make a change to the file. Livereactload appends the version number to the new module, so you lose your breakpoint (good). Let's say you set a breakpoint in this latest file. If you reload the page, you go back to the "unversioned" file from before, so you get your original breakpoint back which is now in the wrong spot (or possibly no breakpoint at all). The correct behavior is to load the breakpoint from the second time you set it. With hashes always being present, this works. I prefer this, even if the URLs look ugly. I think we can find a hash that is small enough, and put it at the end instead of the beginning, and it will help.

[milankinen]

Good points @EugeneZ! 👍

Do you know whether the hash have to be a part of the path (e.g. /file.js/asdfg) or is a query param /file.js?h=asdfg enough?

[EugeneZ]

I don't think it matters. As long as the URL is unique. We don't even need the h=. Just /file.js?abcd will do. For a short hash, we can simply base64 encode the current hash and then truncate it to, say, 4 characters. The base64 is so that we pack as much of the entropy from the hash as possible into those 4 characters. The reason we need very few characters is because we only need the file to be unique for that particular URL, not among all URLs, and the cost of a collision is minor (your source maps don't update until you reload the page), even though collisions are probably going to be very rare anyway. 64 = 6 bits, 6 * 4 = 24 bits (16 million uniques per URL), that seems like plenty.

I would be happy to work on this (it's a simple change), but I'm worried I don't understand @pizza2code 's TypeScript issues. Was the "versionless" change to help with that, or was it just a simplification because you thought the versions weren't needed?

[cguinnup]

Don't worry, @EugeneZ
My PR appended your hash to the source filename as a parameter, like code.ts?hash=a9834f12398b. @milankinen thought it would be even cleaner to make it a version number, and the points you brought up here didn't occur to either of us. The issue with TypeScript was that your code had outright replaced the source filename, with the assumption that source filename == current JS filename. But I fixed it by merely appending to the existing source filename (the one from the sourcemap).

[cguinnup]

As for my two cents, I believe a named parameter like version= or change= makes the hash's appearance a bit clearer to our fellow devs. If I didn't know the idiosyncrasies of LiveReactload + Chrome, I'd be looking at a naked hash in confusion.

[EugeneZ]

@pizza2code Great, thanks for the summary. I will look into making this change tonight. I agree that clarity is better, so I'll use ?version=abcd.

[cguinnup]

Also, I definitely plan to voice my concern about sourcemaps not hot-reloading in the Chromium bug here: https://bugs.chromium.org/p/chromium/issues/detail?id=438251
If this were fixed, we wouldn't need these hashes as a workaround!

[milankinen]

Yeah it was definitely my mistake. @pizza2code's original PR did indeed contain the hash instead of revision number. I've used breakpoints so rarely lately so this use case didn't cross my mind at all.

@EugeneZ your suggestion sounds great! Just make a PR and I'll do my best not to ruin it this time 😄

[EugeneZ]

Sorry for the delay. Got busy with other stuff and this slipped under my radar. I submitted PR #152 which fixes this! Hashes are 6 characters long and added as a ?version=hash at the end of the filename. See PR for more details.

[EugeneZ]

Thanks for merging and releasing 3.3.0 with this change, @milankinen! @pizza2code, would you mind trying it out with your TypeScript setup to make sure I didn't break that again? I am pretty sure I figured out what I did wrong (replacing the filename in the sourcemap instead of appending to it) but just to be sure...