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

Fix instrumentation in shallow rendering #6867

Merged
merged 1 commit into from May 25, 2016

Conversation

sophiebits
Copy link
Collaborator

Previously, this threw:

 FAIL  src/test/__tests__/ReactTestUtils-test.js (7.291s)
● ReactTestUtils › it can fail context when shallowly rendering
  - TypeError: Cannot read property '_source' of null
        at describeID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:70:46)
        at Object.ReactComponentTreeDevtool.getStackAddendumByID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:203:15)
        at checkReactTypeSpec (src/isomorphic/classic/types/checkReactTypeSpec.js:76:58)
        at ReactCompositeComponentMixin._checkContextTypes (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:668:5)
        at ReactCompositeComponentMixin._processContext (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:607:14)
        at ReactCompositeComponentMixin.mountComponent (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:191:30)
        at ReactShallowRenderer._render (src/test/ReactTestUtils.js:483:14)
        at _batchedRender (src/test/ReactTestUtils.js:460:12)
        at ReactDefaultBatchingStrategyTransaction.Mixin.perform (src/shared/utils/Transaction.js:140:20)
        at Object.ReactDefaultBatchingStrategy.batchedUpdates (src/renderers/shared/stack/reconciler/ReactDefaultBatchingStrategy.js:65:19)
        at Object.batchedUpdates (src/renderers/shared/stack/reconciler/ReactUpdates.js:112:20)
        at ReactShallowRenderer.render (src/test/ReactTestUtils.js:453:16)
        at Spec.eval (src/test/__tests__/ReactTestUtils-test.js:289:34)
        at jasmine.Block.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:1067:17)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at jasmine.Queue.start (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2053:8)
        at Spec.jasmine.Spec.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2380:14)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at onComplete (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2096:18)
        at Spec.jasmine.Spec.finish (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2354:5)
        at eval [as onComplete] (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2381:10)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2110:14)
        at eval (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2090:18)
        at Timeout.e [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:440:19)
        at tryOnTimeout (timers.js:224:11)
        at Timer.listOnTimeout (timers.js:198:5)

@gaearon

Previously, this threw:

```
 FAIL  src/test/__tests__/ReactTestUtils-test.js (7.291s)
● ReactTestUtils › it can fail context when shallowly rendering
  - TypeError: Cannot read property '_source' of null
        at describeID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:70:46)
        at Object.ReactComponentTreeDevtool.getStackAddendumByID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:203:15)
        at checkReactTypeSpec (src/isomorphic/classic/types/checkReactTypeSpec.js:76:58)
        at ReactCompositeComponentMixin._checkContextTypes (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:668:5)
        at ReactCompositeComponentMixin._processContext (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:607:14)
        at ReactCompositeComponentMixin.mountComponent (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:191:30)
        at ReactShallowRenderer._render (src/test/ReactTestUtils.js:483:14)
        at _batchedRender (src/test/ReactTestUtils.js:460:12)
        at ReactDefaultBatchingStrategyTransaction.Mixin.perform (src/shared/utils/Transaction.js:140:20)
        at Object.ReactDefaultBatchingStrategy.batchedUpdates (src/renderers/shared/stack/reconciler/ReactDefaultBatchingStrategy.js:65:19)
        at Object.batchedUpdates (src/renderers/shared/stack/reconciler/ReactUpdates.js:112:20)
        at ReactShallowRenderer.render (src/test/ReactTestUtils.js:453:16)
        at Spec.eval (src/test/__tests__/ReactTestUtils-test.js:289:34)
        at jasmine.Block.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:1067:17)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at jasmine.Queue.start (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2053:8)
        at Spec.jasmine.Spec.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2380:14)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at onComplete (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2096:18)
        at Spec.jasmine.Spec.finish (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2354:5)
        at eval [as onComplete] (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2381:10)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2110:14)
        at eval (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2090:18)
        at Timeout.e [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:440:19)
        at tryOnTimeout (timers.js:224:11)
        at Timer.listOnTimeout (timers.js:198:5)
```
@sophiebits
Copy link
Collaborator Author

As a follow-up, I'll look at making this not throw when element is missing.

@gaearon
Copy link
Collaborator

gaearon commented May 25, 2016

As a follow-up, I'll look at making this not throw when element is missing.

👍

This would be consistent with the “sane defaults” we have for other tree devtool getters.

@sophiebits
Copy link
Collaborator Author

Well I was going to make it warn. :) Because I think it should probably never be missing when we want to use it.

@gaearon
Copy link
Collaborator

gaearon commented May 25, 2016

I wonder if we should warn consistently for other getters too? Ideally nobody should be asking about things that don’t exist.

@sophiebits
Copy link
Collaborator Author

That sounds pretty reasonable.

@sophiebits sophiebits added this to the 15.y.0 milestone May 25, 2016
@sophiebits sophiebits merged commit 510155e into facebook:master May 25, 2016
@zpao zpao modified the milestones: 15.y.0, 15-next Jun 1, 2016
zpao pushed a commit to zpao/react that referenced this pull request Jun 8, 2016
Previously, this threw:

```
 FAIL  src/test/__tests__/ReactTestUtils-test.js (7.291s)
● ReactTestUtils › it can fail context when shallowly rendering
  - TypeError: Cannot read property '_source' of null
        at describeID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:70:46)
        at Object.ReactComponentTreeDevtool.getStackAddendumByID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:203:15)
        at checkReactTypeSpec (src/isomorphic/classic/types/checkReactTypeSpec.js:76:58)
        at ReactCompositeComponentMixin._checkContextTypes (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:668:5)
        at ReactCompositeComponentMixin._processContext (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:607:14)
        at ReactCompositeComponentMixin.mountComponent (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:191:30)
        at ReactShallowRenderer._render (src/test/ReactTestUtils.js:483:14)
        at _batchedRender (src/test/ReactTestUtils.js:460:12)
        at ReactDefaultBatchingStrategyTransaction.Mixin.perform (src/shared/utils/Transaction.js:140:20)
        at Object.ReactDefaultBatchingStrategy.batchedUpdates (src/renderers/shared/stack/reconciler/ReactDefaultBatchingStrategy.js:65:19)
        at Object.batchedUpdates (src/renderers/shared/stack/reconciler/ReactUpdates.js:112:20)
        at ReactShallowRenderer.render (src/test/ReactTestUtils.js:453:16)
        at Spec.eval (src/test/__tests__/ReactTestUtils-test.js:289:34)
        at jasmine.Block.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:1067:17)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at jasmine.Queue.start (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2053:8)
        at Spec.jasmine.Spec.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2380:14)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at onComplete (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2096:18)
        at Spec.jasmine.Spec.finish (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2354:5)
        at eval [as onComplete] (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2381:10)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2110:14)
        at eval (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2090:18)
        at Timeout.e [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:440:19)
        at tryOnTimeout (timers.js:224:11)
        at Timer.listOnTimeout (timers.js:198:5)
```
(cherry picked from commit 510155e)
zpao pushed a commit that referenced this pull request Jun 14, 2016
Previously, this threw:

```
 FAIL  src/test/__tests__/ReactTestUtils-test.js (7.291s)
● ReactTestUtils › it can fail context when shallowly rendering
  - TypeError: Cannot read property '_source' of null
        at describeID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:70:46)
        at Object.ReactComponentTreeDevtool.getStackAddendumByID (src/renderers/shared/devtools/ReactComponentTreeDevtool.js:203:15)
        at checkReactTypeSpec (src/isomorphic/classic/types/checkReactTypeSpec.js:76:58)
        at ReactCompositeComponentMixin._checkContextTypes (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:668:5)
        at ReactCompositeComponentMixin._processContext (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:607:14)
        at ReactCompositeComponentMixin.mountComponent (src/renderers/shared/stack/reconciler/ReactCompositeComponent.js:191:30)
        at ReactShallowRenderer._render (src/test/ReactTestUtils.js:483:14)
        at _batchedRender (src/test/ReactTestUtils.js:460:12)
        at ReactDefaultBatchingStrategyTransaction.Mixin.perform (src/shared/utils/Transaction.js:140:20)
        at Object.ReactDefaultBatchingStrategy.batchedUpdates (src/renderers/shared/stack/reconciler/ReactDefaultBatchingStrategy.js:65:19)
        at Object.batchedUpdates (src/renderers/shared/stack/reconciler/ReactUpdates.js:112:20)
        at ReactShallowRenderer.render (src/test/ReactTestUtils.js:453:16)
        at Spec.eval (src/test/__tests__/ReactTestUtils-test.js:289:34)
        at jasmine.Block.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:1067:17)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at jasmine.Queue.start (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2053:8)
        at Spec.jasmine.Spec.execute (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2380:14)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2100:31)
        at onComplete (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2096:18)
        at Spec.jasmine.Spec.finish (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2354:5)
        at eval [as onComplete] (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2381:10)
        at jasmine.Queue.next_ (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2110:14)
        at eval (node_modules/jest-jasmine1/vendor/jasmine-1.3.0.js:2090:18)
        at Timeout.e [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:440:19)
        at tryOnTimeout (timers.js:224:11)
        at Timer.listOnTimeout (timers.js:198:5)
```
(cherry picked from commit 510155e)
@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.

None yet

3 participants