Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Discussion: Re-architect DevTools #1214

Closed
2 of 4 tasks
bvaughn opened this issue Nov 2, 2018 · 8 comments
Closed
2 of 4 tasks

Discussion: Re-architect DevTools #1214

bvaughn opened this issue Nov 2, 2018 · 8 comments

Comments

@bvaughn
Copy link
Contributor

bvaughn commented Nov 2, 2018

The problem

React DevTools currently support all "recent" versions of React (14+? 13+?). As the React API grows and changes, this will become increasingly difficult to maintain. Several recent changes to React core have broken parts of DevTools (e.g. the profiler) without us even being aware.

DevTools exists as a separate project because it supports multiple versions of React. However this makes automated testing and type-checking more difficult (see #1209).

Possible solution

  1. Extract most of the current "backend" and "frontend" code pieces into a new package, e.g. react-devtools-interface, and move this package to the React repo..
  2. Release an initial version of react-devtools-interface that supports all of the currently supported versions of React (14+? 13+?).
  3. Remove all legacy code from react-devtools-interface so that it only supports a single version of React (e.g. the version it corresponds to in Git).
  4. Reduce the react-devtools project to be a basic shell that detects which version(s) of React are currently present, and loads the corresponding react-devtools-interface package(s) at runtime1.

1: Actual runtime loaded code seems to be frowned upon by both Google and Mozilla, but we could bundle the needed versions of react-devtools-interface along with the shell extension and have it swap between them based on the injected React version(s).

Benefits

  1. Size and complexity of DevTools drastically decreases.
  2. The cost of refactoring the React API and its internals decreases as a result.
  3. Moving react-devtools-interface to the React repo would us to have automated integration tests and Flow type checks.
  4. Reducing complexity in the DevTools interface likely also results in improved performance.
  5. This offers us a nice opportunity to address longstanding tech debt in some older sections of DevTools.

Drawbacks

  1. Using DevTools with multiple versions of React would result in a larger download size for the extension. (We should be able to easily remain below the 200 MB size limit though.)

Open questions

  • How would we support bug fixes (or version-agnostic additions) to DevTools if this required multiple packages to be updated and released? (Could we automated this somehow?)
  • How would we support multiple versions of React on a single page? The shell wrapper would require some minimal UI for its loading state anyway. Maybe this UI could also handle multiple React versions by prompting you to select which version you want to inspect (and enable you to toggle this version later somehow)?
  • How would this lazily-loaded package work for local development? (How would you iterate on it?) npm link the local react-devtools-interface package while developing.
  • What would this mean for React-alike frameworks like Preact?
@captbaritone
Copy link

I recently read that Chrome is going to start raising the bar for extensions that load remote js. I wouldn’t be too surprised if they continue in that direction.

Changes to the extensions review process

Going forward, extensions that request powerful permissions will be subject to additional compliance review. We’re also looking very closely at extensions that use remotely hosted code, with ongoing monitoring.

From https://blog.chromium.org/2018/10/trustworthy-chrome-extensions-by-default.html?m=1

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 4, 2018

Thanks @captbaritone. This is exactly the sort of thing I'm hoping to rule out before going too far down this path. I know who to talk to at Mozilla but I'm not sure yet who to reach out to at Google.

I'll try reaching out to Nicole to see if she can refer me to someone for clarification.

@ngyikp
Copy link
Contributor

ngyikp commented Nov 4, 2018

Why not just bundle all the version scripts into the extension package so no external download is needed? It'll also solve the offline problem.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 4, 2018

The react-devtools-interface package I'm envisioning would include both the "backend" code like attachRendererFiber as well as the "frontend" UI for DevTools. So packaging every version would make the extension pretty large over time. But yeah, I suppose that is an option worth considering if runtime loaded code ends up being frowned upon too much.

@HaNdTriX
Copy link

HaNdTriX commented Nov 5, 2018

I recommend bundling all versions in the extension package and then only load the needed versions lazily into the runtime.

We’re also looking very closely at extensions that use remotely hosted code, with ongoing monitoring. Your extension’s permissions should be as narrowly-scoped as possible, and all your code should be included directly in the extension package, to minimize review time.

Source: https://blog.chromium.org/2018/10/trustworthy-chrome-extensions-by-default.html

@captainbrosset
Copy link

I just discussed with an AMO (addons.mozilla.org) reviewer and they said that loading and executing remote code would violate the AMO policies.
So I also recommend bundling all versions in the package, otherwise the extension will probably just get rejected at review time.

@bvaughn
Copy link
Contributor Author

bvaughn commented Nov 6, 2018

Thanks everyone! I'll update the proposal.

My only lingering concern was whether we would run into any extension size limits down the road. But a quick search suggests that the size limit is pretty high (200 MB) so that shouldn't be a concern for us. 😄

@bvaughn
Copy link
Contributor Author

bvaughn commented Aug 19, 2019

React DevTools has been rewritten and recently launched a new version 4 UI. The source code for this rewrite was done in a separate repository and now lives in the main React repo (github.com/facebook/react).

Because version 4 was a total rewrite, and all issues in this repository are related to the old version 3 of the extension, I am closing all issues in this repository. If you can still reproduce this issue, or believe this feature request is still relevant, please open a new issue in the React repo: https://github.com/facebook/react/issues/new?labels=Component:%20Developer%20Tools

@bvaughn bvaughn closed this as completed Aug 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants