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.

Sign up for GitHub

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

[Fiber] Split dev methods within ReactComponentTreeHook #9096

Merged
merged 13 commits into from
Mar 7, 2017
Merged

[Fiber] Split dev methods within ReactComponentTreeHook #9096

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0e23042
refactor of ReactComponentTreeHook to isolate dev methods
trueadm Mar 2, 2017
e8f3f92
fixed flow issues and bug resulting in failing tests
trueadm Mar 2, 2017
da9d918
WIP - not sure how to get the Flow type working properly
trueadm Mar 2, 2017
54fef50
re-added missing files
trueadm Mar 2, 2017
1f7cbb6
updated flow annotations
trueadm Mar 2, 2017
9a1db49
removed comment
trueadm Mar 2, 2017
397fd2b
removed double semicolon
trueadm Mar 2, 2017
91de17e
added guards in place to prevent mixed prod+dev errors from occuring
trueadm Mar 2, 2017
b572e3c
moved non DEV methods to a new file called ReactFiberComponentTreeHoo…
trueadm Mar 6, 2017
b67a29a
Merge remote-tracking branch 'upstream/master' into component-tree-ho…
trueadm Mar 6, 2017
af4d249
fixes tests so they correctly pass in fiber
trueadm Mar 6, 2017
f992d54
removed describe.only
trueadm Mar 6, 2017
a159e85
removed inline boolean checks for methods
trueadm Mar 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/isomorphic/classic/element/ReactDebugCurrentFrame.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@

import type { Fiber } from 'ReactFiber';
import type { DebugID } from 'ReactInstanceType';
import type { ComponentTreeHookType } from '../../hooks/ReactComponentTreeHook';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should just say from 'ReactComponentTreeHook'?


const ReactDebugCurrentFrame = {};

if (__DEV__) {
var {
const {
getStackAddendumByID,
getStackAddendumByWorkInProgressFiber,
getCurrentStackAddendum,
} = require('ReactComponentTreeHook');

}: ComponentTreeHookType = require('ReactComponentTreeHook');
// Component that is being worked on
ReactDebugCurrentFrame.current = (null : Fiber | DebugID | null);

Expand All @@ -38,7 +38,9 @@ if (__DEV__) {
if (typeof current === 'number') {
// DebugID from Stack.
const debugID = current;
stack = getStackAddendumByID(debugID);
if (getStackAddendumByID) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to not do this since we already know we're in DEV. See my comment below on potential way to accomplish this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check seems unnecessary, other than for flow, since we're within a __DEV__ block. I liked Dan's suggestion below- to use a different type for dev/prod so we can avoid all of the unnecessary not-defined checks below.

stack = getStackAddendumByID(debugID);
}
} else if (typeof current.tag === 'number') {
// This is a Fiber.
// The stack will only be correct if this is a work in progress
Expand All @@ -47,7 +49,9 @@ if (__DEV__) {
stack = getStackAddendumByWorkInProgressFiber(workInProgress);
}
} else if (element !== null) {
stack = getCurrentStackAddendum(element);
if (getCurrentStackAddendum) {
stack = getCurrentStackAddendum(element);
}
}
return stack;
};
Expand Down
Loading