React: Why does DevTool Chrome Extension need access to history?

[luanmuniz]

I woke up today and the DevTool Extension for Chrome asked for additional permissions. More specifically access to history.

I couldn't find any explanations or reference in the changelog, so I opened this issue.

[bvaughn]

Why does DevTool Chrome Extension need access to history?

It doesn't.

Permissions changed initially with v4.0.0 because I thought I needed some additional ones for some new features, but after realizing what the upgrade experience was like for these, I removed them and refactored the code. v4.0.2 (latest in the Chrome and Firefox stores) requires no new permissions.

If you're curious why they were there in the first place: there were three feature areas that related to new APIs, all related to the Profiler.

Save profiling data between navigations

Like Chrome's own profiler, DevTools remembers profiling data between page loads. (This helps in the event that there's an error, or an unexpected reload b'c of hot reloading.) Originally I needed used a new API to listen for an event just before navigation so that I could store the current data in memory so as not to lose it after the reload. I refactored this code to remove the dependency on that new API.

Import/export (share) profiling data

Version 4 lets you export and import profile data to enable sharing. I originally built this using the Chrome chrome.downloads.download API but that actually turned out to be buggy. So I reported the bugs to the Chrome DevTools team, and replaced it with a simpler JavaScript file download. (It doesn't let you choose the file name, but it doesn't require any special permissions.)

Profiling screenshots

For a while, I added an opt-in feature to the Profiler that took screenshots of each commit making use of the chrome.tabCapture.capture API. This turned out to be a pretty slow API though, so I had to throttle it- which in turn made it less useful, since many commits would be "skipped" (no screenshots). So I decided to just yank it out and remove the permission.

[bvaughn]

Let's keep chatting here, but since this is a question and has been answered, I'm going to close the issue.

[gaearon]

To be super clear for people who don’t read longer posts, 4.0.2 removes those permissions.

[luanmuniz]

This kind of information should be available in the changelog, right?
Both insertion and removal of new permissions.

Is there any reference for those apis uses anywhere? I tried to search for it before opening this issue, but I didn't find any. I couldn't find the source code for the Chrome Extension to look for where it was being used.

It seems that I'm missing something.

[bvaughn]

@luanmuniz CHANGELOG is not exhaustive. Just tried to touch on the main points. Since the permissions ended up being a non-change (after a quick follow up point release) I don't think they need to be added to it.

Is there any reference for those apis uses anywhere

Other than my comment above? Just in the git repo history.

I couldn't find the source code for the Chrome Extension to look for where it was being used.

Right now it's here: https://github.com/bvaughn/react-devtools-experimental/tree/source

We are in the process of merging it into this repo (#16381) but there's a couple of small blockers that I need to work out before it lands.

[gaearon]

This kind of information should be available in the changelog, right?

I did add it to the changelog yesterday: https://github.com/facebook/react/commit/c1d3f7f1a97adad9441287a92dcd4ac5d2478c38

We also tweeted about this immediately after we discovered the problem and uploaded the fix: https://twitter.com/reactjs/status/1162174507391574017. That was before this issue was even filed.

I’m sorry we forgot the update the CHANGELOG immediately. Fixing this was pretty stressful, it was 2:30am in my timezone, and I just forgot to do it.

To be clear though, this is a complete rewrite of the extension. It took many months. We didn’t intentionally try to sneak in new permissions in the existing code if that’s what you’re implying. As soon as we discovered that the new version unnecessarily had broader permissions, we did two more releases to fix the issue.

See also my HN comment: https://news.ycombinator.com/reply?id=20711416&goto=threads%3Fid%3Ddanabramov%2320711416

Again, I’m sorry we screwed this up. Chrome doesn’t show the permission dialog in development so we only realized they changed after doing the release to two million users. I hope you can empathize with this being a stressful mistake to fix.

[gaearon]

Is there any reference for those apis uses anywhere?

Yes.

Two new permissions were completely unused: https://github.com/bvaughn/react-devtools-experimental/commit/545de6f02e7d8905ba4e1cd358e68bb640a31a0a. They were originally added for new Profiler features, as explained in the above comment. Did you get a chance to read it? Here’s a link in case you missed it: https://github.com/facebook/react/issues/16421#issuecomment-522037725. As explained in the same comment, we ended up removing these two features but forgot to update the permissions. 4.0.1 fixed that.

However, there was one remaining new permission, as we realized after 4.0.1 went live. It turned out that we misunderstood the API, and didn’t actually need it. This was fixed in https://github.com/bvaughn/react-devtools-experimental/commit/527fc4a63f497db1f370a8312807c2a68147edec and went live in 4.0.2.

I’m sorry we weren’t diligent about the changelog. I’m not sure if you noticed, but 4.0.0 is the first DevTools release to even have a changelog entry. In the past we’d usually fix bugs and push the updates without a changelog because most extension users never even visit the GitHub repository. I don’t know if we’ll commit to adding notes for every release now or stick to doing it for big releases like this one. But I hope this clarifies your questions. If not please let me know how I can help. Thanks!

[bvaughn]

Thank you for all the added context, Dan.

I've also gone back and filled in the changelog for 4.0.1 and 4.0.2 a little to add more context about which permissions were removed with each update:
https://github.com/facebook/react/blob/master/packages/react-devtools/CHANGELOG.md#402-august-15-2019

[luanmuniz]

I hope you can empathize with this being a stressful mistake to fix.

Yes, of course. The fact that you guys removed quickly after the occurrence shows that the good intention is there. I can only imagine how hard it is to take care of a project the size of these ones.

I was just trying to make sense of it all. Since did find the changelog, but not the source code, I was thinking that it could be that it was a private project, so that's the reason I asked about the reference, to make sure I was asking the right questions.

Thank you for all the added context, Dan.

The extra context is very helpful, I didn't know that the bvaughn/react-devtools-experimental repo existed in the first place. It is the first time I'm looking at this project.

I've also gone back and filled in the changelog for 4.0.1 and 4.0.2

Thanks.

I’m not sure if you noticed, but 4.0.0 is the first DevTools release to even have a changelog entry.

No, I didn't hahah
I actually searched in Google for the repo and it pointed me to this one.

But I hope this clarifies your questions. If not please let me know how I can help.

Yes, it does. Thanks for the answers guys!

[gaearon]

Yeah, we didn’t want to emphasize the old repo because we’re moving the code into this one, and so want it to become the source of truth. Hopefully we’ll get it done soon, but there’s still more work to do there.