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

Expose devtools hook injection to other consumers on React object #2797

Closed
gaearon opened this issue Jan 1, 2015 · 13 comments
Closed

Expose devtools hook injection to other consumers on React object #2797

gaearon opened this issue Jan 1, 2015 · 13 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Jan 1, 2015

Currently React looks for __REACT_DEVTOOLS_GLOBAL_HOOK__ global and calls inject on it, if it exists. This works fine if there's one listener (e.g. Chrome React DevTools themselves).

I'd love it if React supported arbitrary number of independent hooks. In React Hot Loader I'm reaching into internals (I need ReactMount to get all mounted instances) but I wish I could do that without require-ing an internal module.

Ideally I wish React exposed the same modules that are available to global hooks, via some kind of React.registerDeveloperTool(). This would be called automatically for __REACT_DEVTOOLS_GLOBAL_HOOK__ but available on React object if I want to do it from a library. (I don't mean production libraries, but debugging aid libraries such as React Hot Loader and potentially others.)

@gaearon gaearon changed the title Expose devtools hook injection Expose devtools hook injection to other consumers on React object Jan 1, 2015
@sebmarkbage
Copy link
Collaborator

One of the reasons we added it was because React may not load immediately and many tools wants to be notified once React is loaded an available. There might also be multiple Reacts on a single page or at least in multiple iframes.

The idea is that other tools can override and chain the __REACT_DEVTOOLS_GLOBAL_HOOK__ .inject hook by running after the devtools but before React is loaded.

@gaearon
Copy link
Collaborator Author

gaearon commented Jan 6, 2015

Chaining would be cool but it doesn't work now this way, does it? Once DevTools hook ran, you can't have your inject called somehow; moreover DevTools define it as non-writeable.

Any way we can change hook API to explicitly support chaining?

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 28, 2015

Never mind!
I started working on the next version of React Hot Loader, and monkeypatching is more convenient :-).

@gaearon gaearon closed this as completed Apr 28, 2015
@sebmarkbage
Copy link
Collaborator

Monkey patching internals will break as soon as we start precompiling React using Closure Compiler so you'll need officially supported hooks. Just like the devtools and ReactPerf etc. will also need.

On Apr 29, 2015, at 12:41 AM, Dan Abramov [email protected] wrote:

Never mind!
I started working on the next version of React Hot Loader, and monkeypatching is more convenient :-).


Reply to this email directly or view it on GitHub.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2015

No, I'm not talking about monkeypatching React internals. I'm monkeypatching user's components. Because they pass through my transform anyway, in addition to method forwarding, I hijack componentWillMount and componentWillUnmount to track instances. This proves to be enough in my case. Also, this is only going to be enabled in development, so I suspect in this case I'm getting a “source” React and not precompiled one anyway.

@ghost
Copy link

ghost commented Apr 30, 2015

@gaearon Earlier just this morning I put together what I believe @sebmarkbage was talking about with the chaining. For my particular use-case I'm using your react-hot-api to automatically hot reload all ReactClasses when in a development environment. It monkey-patches React.createClass (and I'm not concerned about minification issues here since it's the dev environment), and either later today or tomorrow I'll make sure it works for ES6 classes. (Will I have to do anything extra there or will that use the same React.createClass?)

On a personal note, I really didn't like what I had to do to make this work, since the powers that be seem to be opposed to exposing ReactMount or _instancesByReactRootID (see #3568 which I'm sure @gaearon remembers). It's pretty ugly (more monkey-patching yay!) and must be applied before React is ever imported, but it seems to work both with and without react-devtools.

https://gist.github.com/timbur/5317a6079a585db253a4

I suppose this might be a "solution" to your source problem, but I agree that in the future we will probably need something more elegant like you described in your original post.

That React... so hot right now.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2015

@timbur Have you looked at react-hotify yet? It has a more succinct API than react-hot-api and does not require you to pass ReactMount ugliness. It does not currently support createClass-style classes (only ES6 ones) but that should not be hard to port over. What do you think?

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2015

@timbur

How do you solve the problem of per-component makeHot? For it to work, separate versions of makeHot need to be kept for each component. For example, if you have class A and class B, each needs to get its own makeHot, so when class A reloads, the original class A is patched, and similarly, when class B reloads, its own makeHot patches the original class B. In your Gist, it looks like a reload would overwrite all classes in the application.

This is made more explicit in react-hotify API: it keeps an internal map of “hotifying” functions per component class, and you are required to pass a string to it uniquely identifying the component. What usually works fairly well is filename plus component name.

@ghost
Copy link

ghost commented Apr 30, 2015

@gaearon Thanks for the heads up about each class needing their own makeHot. I just updated the gist to fix that.

I just checked out react-hotify and I'm liking it! It looks pretty straightforward and it makes more sense than react-hot-api. Decorators have grown on me a lot over the past few months (I use them for almost everything now) so I'm glad to see they're becoming widely adopted and soon to be standardized.

Upon first glance it looks like it would be pretty easy to make react-hotify work for createClass, and babel-plugin-react-hotify looks perfect for my use-case. I can throw out that gist, and when I get a chance I'll see about incorporating react-hotify into what I'm working on. Thanks for this!

filename plus component name

Semi-unrelated, regarding that: Anyone know if __filename is for sure expected to be part of the module standard? I saw a short dicussion on it a while back and it looks like it will be but not yet solidified.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 30, 2015

@timbur

I just updated the gist to fix that.

I think it's still slightly wrong because those makeHot functions need to be reused between different versions of the same component. The way patching works, the same makeHot function should handle both the original version of the component, and all its updated “versions” which you get from the re-evaluated module code on hot updates. This is how it knows that they all refer to the same class in code, and sets up the patching. So in your case you need to somehow store a map of makeHot functions based on some unique class identifier. How you derive that identifier depends on the build system and tools at your disposal.

Upon first glance it looks like it would be pretty easy to make react-hotify work for createClass

If you implement it, can I ask you to PR it to react-hotify? Ideally I'd like it to be more of a team effort than it is now :-). It's got plenty of tests so they should give you an idea of how to test the API. It would be sensible to add a test case using createClass to each test file. It won't work right away: createClass has auto-binding, and new methods after hot reload need to be auto-bound. This should be possible to port over from this function in react-hot-api. Etc.

PS Maybe you could open an issue in react-hotify so we don't spam people here.

@skirankumar7
Copy link

What REACT_DEVTOOLS_GLOBAL_HOOK.inject method does?
How it is useful?
please comment

@Venryx
Copy link

Venryx commented Nov 16, 2019

@skirankumar7 You may find these two source files helpful for understanding what data the hook injection site makes available:

function inject(renderer) {

function handleCommitFiberRoot(root, priorityLevel) {

@Venryx
Copy link

Venryx commented Nov 16, 2019

It's four years later; is there some sort of API available now for third party extensions to use the same __REACT_DEVTOOLS_GLOBAL_HOOK__ injection site?

I notice here that when react-devtools places its hook at that site, it marks the property as non-configurable: https://github.com/facebook/react/blob/master/packages/react-devtools-shared/src/hook.js#L305

This is unfortunate as it means no other extensions are able place their own patched/intercept version of the hook there.

Granted, there is a workaround: while developers of other extensions can't patch/intercept the hook variable itself, we can patch all the properties on the hook object.

We can do something like:

const hook = __REACT_DEVTOOLS_GLOBAL_HOOK__;
const oldHook = {...hook};
hook.onCommitFiberRoot = (...args)=> {
    // custom code here
    oldHook.onCommitFiberRoot(...args);
};

Is the above the best way currently available, or is there a newer way I'm not aware of?

EDIT

Complete example for others: (works regardless of which extension runs first)

let hook = {
	onCommitFiberRoot: (rendererID, root, priorityLevel)=> {
		// custom code here
		if (officialHook) officialHookProps.onCommitFiberRoot(rendererID, root, priorityLevel);
	},
	onCommitFiberUnmount: (rendererID, fiber)=> {
		// custom code here
		if (officialHook) officialHookProps.onCommitFiberUnmount(rendererID, fiber);
	},
};

let officialHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
let officialHookProps = {...officialHook};

// wait until the official hook is added, then patch it
let startTime = Date.now();
let timerID = setInterval(()=> {
	// cancel wait if in prod, and we've waited for more than 5s
	if (!__DEV__ && Date.now() - startTime > 5000) {
		return void clearInterval(timerID);
	}

	if (window.__REACT_DEVTOOLS_GLOBAL_HOOK__ && window.__REACT_DEVTOOLS_GLOBAL_HOOK__ != hook) {
		officialHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
		officialHookProps = {...officialHook};
		//Object.assign(officialHook, hook);
		for (let [key, value] of Object.entries(hook)) {
			// only transfer functions
			if (typeof value == "function") {
				officialHook[key] = value;
			}
		}
		// we've patched the official hook (provided by react-devtools), so we're done
		return void clearInterval(timerID);
	}
}, 100);

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

No branches or pull requests

4 participants