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

Move instrumentation to renderers/shared. Closes #6797 #6801

Merged
merged 1 commit into from
May 19, 2016

Conversation

iamdustan
Copy link
Contributor

From the initial report I also moved ReactPerf out, which maybe should be moved back? Also you may want me to move some files around, yet. The following is the git log version of the moves:

 rename src/{isomorphic => shared}/ReactPerf.js (100%)
 rename src/{isomorphic => shared}/__tests__/ReactPerf-test.js (100%)
 rename src/{isomorphic => shared}/devtools/ReactComponentTreeDevtool.js (100%)
 rename src/{isomorphic => shared}/devtools/ReactHostOperationHistoryDevtool.js (100%)
 rename src/{isomorphic => shared}/devtools/ReactInvalidSetStateWarningDevTool.js (100%)
 rename src/{isomorphic => shared}/devtools/__tests__/ReactComponentTreeDevtool-test.js (100%)
 rename src/{isomorphic => shared}/devtools/__tests__/ReactComponentTreeDevtool-test.native.js (100%)
 rename src/{isomorphic => shared}/devtools/__tests__/ReactHostOperationHistoryDevtool-test.js (100%)
 rename src/{isomorphic => shared/utils}/ReactDebugTool.js (100%)
 rename src/{isomorphic => shared/utils}/ReactInstrumentation.js (100%)
 rename src/{isomorphic => shared/utils}/__tests__/ReactDebugTool-test.js (100%)

But it may be easier to see the added files in this treeview (with only added files kept):

src/shared
├── ReactPerf.js
├── __tests__
│   └── ReactPerf-test.js
├── devtools
│   ├── ReactComponentTreeDevtool.js
│   ├── ReactHostOperationHistoryDevtool.js
│   ├── ReactInvalidSetStateWarningDevTool.js
│   └── __tests__
│       ├── ReactComponentTreeDevtool-test.js
│       ├── ReactComponentTreeDevtool-test.native.js
│       └── ReactHostOperationHistoryDevtool-test.js
└── utils
    ├── ReactDebugTool.js
    ├── ReactInstrumentation.js
    └── __tests__
          └── ReactDebugTool-test.js

'setState(...): You passed an undefined or null state object; ' +
'instead, use forceUpdate().'
);
}
this.updater.enqueueSetState(this, partialState);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.updater falls back to the ReactNoopUpdateQueuewhich is also in isomorphic/modern/class/. Is it okay to not call ReactInstrumentation.debugTool.onSetState() in that scenario?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it okay to not call ReactInstrumentation.debugTool.onSetState() in that scenario?

Yea, I think it’s fine.

@gaearon
Copy link
Collaborator

gaearon commented May 19, 2016

Hmm. Doesn’t this move to src/shared rather than src/renderers/shared?

@iamdustan
Copy link
Contributor Author

Doh. Updated.

Git view

 rename src/{isomorphic => renderers/shared}/ReactDebugTool.js (100%)
 rename src/{isomorphic => renderers/shared}/ReactInstrumentation.js (100%)
 rename src/{isomorphic => renderers/shared}/ReactPerf.js (100%)
 rename src/{isomorphic => renderers/shared}/__tests__/ReactDebugTool-test.js (100%)
 rename src/{isomorphic => renderers/shared}/__tests__/ReactPerf-test.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/ReactComponentTreeDevtool.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/ReactHostOperationHistoryDevtool.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/ReactInvalidSetStateWarningDevTool.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/__tests__/ReactComponentTreeDevtool-test.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/__tests__/ReactComponentTreeDevtool-test.native.js (100%)
 rename src/{isomorphic => renderers/shared}/devtools/__tests__/ReactHostOperationHistoryDevtool-test.js (100%)

(Minimal) Tree view

src/renderers/shared
├── ReactDebugTool.js
├── ReactInstrumentation.js
├── ReactPerf.js
├── __tests__
│   ├── ReactDebugTool-test.js
│   └── ReactPerf-test.js
└── devtools
    ├── ReactComponentTreeDevtool.js
    ├── ReactHostOperationHistoryDevtool.js
    ├── ReactInvalidSetStateWarningDevTool.js
    └── __tests__
        ├── ReactComponentTreeDevtool-test.js
        ├── ReactComponentTreeDevtool-test.native.js
        └── ReactHostOperationHistoryDevtool-test.js

@ghost
Copy link

ghost commented May 19, 2016

@iamdustan updated the pull request.

@gaearon gaearon merged commit d125682 into facebook:master May 19, 2016
@zpao zpao added this to the 15-next milestone Jun 1, 2016
zpao pushed a commit to zpao/react that referenced this pull request Jun 8, 2016
…ared

Move instrumentation to renderers/shared. Closes facebook#6797
(cherry picked from commit d125682)
zpao pushed a commit that referenced this pull request Jun 14, 2016
Move instrumentation to renderers/shared. Closes #6797
(cherry picked from commit d125682)
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants