Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 1 addition & 6 deletions agent/Agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var {EventEmitter} = require('events');

var assign = require('object-assign');
var guid = require('../utils/guid');
var getIn = require('./getIn');

import type {RendererID, DataType, OpaqueNodeHandle, NativeType, Helpers} from '../backend/types';

Expand Down Expand Up @@ -406,10 +407,4 @@ class Agent extends EventEmitter {
}
}

function getIn(base, path) {
return path.reduce((obj, attr) => {
return obj ? obj[attr] : null;
}, base);
}

module.exports = Agent;
24 changes: 17 additions & 7 deletions agent/Bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
var consts = require('./consts');
var hydrate = require('./hydrate');
var dehydrate = require('./dehydrate');
var getIn = require('./getIn');
var performanceNow = require('fbjs/lib/performanceNow');

// Custom polyfill that runs the queue with a backoff.
Expand Down Expand Up @@ -396,15 +397,30 @@ class Bridge {

_inspectResponse(id: string, path: Array<string>, callback: number) {
var inspectable = this._inspectables.get(id);

var result = {};
var cleaned = [];
var proto = null;
var protoclean = [];

if (inspectable) {
var val = getIn(inspectable, path);
var protod = false;
var isFn = typeof val === 'function';

if (val && typeof val[Symbol.iterator] === 'function') {
var iterVal = Object.create({}); // flow throws "object literal incompatible with object type"
var count = 0;
for (const entry of val) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this will get fairly unhappy for long lists. Can we bail after ~100 and indicate that there's more we're not showing?

Copy link
Contributor

Choose a reason for hiding this comment

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

this can probably be done as a separate diff, I just would rather an extension update not make people's apps suddenly really slow under debugging

if (count > 100) {
// TODO: replace this if block with better logic to handle large iterables
break;
}
iterVal[count] = entry;
count++;
}
val = iterVal;
}

Object.getOwnPropertyNames(val).forEach(name => {
if (name === '__proto__') {
protod = true;
Expand Down Expand Up @@ -438,10 +454,4 @@ class Bridge {
}
}

function getIn(base, path) {
return path.reduce((obj, attr) => {
return obj ? obj[attr] : null;
}, base);
}

module.exports = Bridge;
4 changes: 2 additions & 2 deletions agent/__tests__/dehydrate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('dehydrate', () => {
var cleaned = [];
var result = dehydrate(object, cleaned);
expect(cleaned).toEqual([['a', 'b', 'c']]);
expect(result.a.b.c).toEqual({type: 'object', name: '', meta: null});
expect(result.a.b.c).toEqual({type: 'object', name: '', meta: {}});
});

it('cleans a deeply nested array', () => {
Expand All @@ -53,6 +53,6 @@ describe('dehydrate', () => {
var result = dehydrate(object, cleaned);
expect(cleaned).toEqual([['a', 'b', 'c'], ['a', 'b', 'd']]);
expect(result.a.b.c).toEqual({type: 'array', name: 'Array', meta: {length: 2}});
expect(result.a.b.d).toEqual({type: 'object', name: 'Something', meta: null});
expect(result.a.b.d).toEqual({type: 'object', name: 'Something', meta: {}});
});
});
160 changes: 112 additions & 48 deletions agent/dehydrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,61 @@
*/
'use strict';

/**
* Get a enhanced/artificial type string based on the object instance
*/
function getPropType(data: Object): string | null {
if (!data) {
return null;
}
var type = typeof data;

if (type === 'object') {
if (data._reactFragment) {
return 'react_fragment';
}
if (Array.isArray(data)) {
return 'array';
}
if (ArrayBuffer.isView(data)) {
if (data instanceof DataView) {
return 'data_view';
}
return 'typed_array';
}
if (data instanceof ArrayBuffer) {
return 'array_buffer';
}
if (data[Symbol.iterator] === 'function') {
return 'iterator';
}
}

return type;
}

/**
* Generate the dehydrated metadata for complex object instances
*/
function createDehydrated(type: string, data: Object, cleaned: Array<Array<string>>, path: Array<string>): Object {
var meta = {};

if (type === 'array' || type === 'typed_array') {
meta.length = data.length;
}
if (type === 'iterator' || type === 'typed_array') {
meta.readOnly = true;
}

cleaned.push(path);

return {
type,
meta,
name: !data.constructor || data.constructor.name === 'Object' ? '' : data.constructor.name,
};
}

/**
* Strip out complex data (instances, functions, and data nested > 2 levels
* deep). The paths of the stripped out objects are appended to the `cleaned`
Expand All @@ -30,63 +85,72 @@
* }
* and cleaned = [["some", "attr"], ["other"]]
*/
function dehydrate(data: Object, cleaned: Array<Array<string>>, path?: Array<string>, level?: number): string | Object {
level = level || 0;
path = path || [];
if (typeof data === 'function') {
cleaned.push(path);
return {
name: data.name,
type: 'function',
};
}
if (!data || typeof data !== 'object') {
if (typeof data === 'string' && data.length > 500) {
return data.slice(0, 500) + '...';
}
function dehydrate(data: Object, cleaned: Array<Array<string>>, path?: Array<string> = [], level?: number = 0): string | Object {

var type = getPropType(data);

switch (type) {

case 'function':
cleaned.push(path);
return {
name: data.name,
type: 'function',
};

case 'string':
return data.length <= 500 ? data : data.slice(0, 500) + '...';

// We have to do this assignment b/c Flow doesn't think "symbol" is
// something typeof would return. Error 'unexpected predicate "symbol"'
var type = typeof data;
if (type === 'symbol') {
case 'symbol':
cleaned.push(path);
return {
type: 'symbol',
name: data.toString(),
};
}
return data;
}
if (data._reactFragment) {

// React Fragments error if you try to inspect them.
return 'A react fragment';
}
if (level > 2) {
cleaned.push(path);
return {
type: Array.isArray(data) ? 'array' : 'object',
name: !data.constructor || data.constructor.name === 'Object' ? '' : data.constructor.name,
meta: Array.isArray(data) ? {
length: data.length,
} : null,
};
}
if (Array.isArray(data)) {
// $FlowFixMe path is not undefined.
return data.map((item, i) => dehydrate(item, cleaned, path.concat([i]), level + 1));
}
// TODO when this is in the iframe window, we can just use Object
if (data.constructor && typeof data.constructor === 'function' && data.constructor.name !== 'Object') {
cleaned.push(path);
return {
name: data.constructor.name,
type: 'object',
};
}
var res = {};
for (var name in data) {
res[name] = dehydrate(data[name], cleaned, path.concat([name]), level + 1);
case 'react_fragment':
return 'A React Fragment';

// ArrayBuffers error if you try to inspect them.
case 'array_buffer':
case 'data_view':
cleaned.push(path);
return {
type,
name: type === 'data_view' ? 'DataView' : 'ArrayBuffer',
meta: {
length: data.byteLength,
uninspectable: true,
},
};

case 'array':
if (level > 2) {
return createDehydrated(type, data, cleaned, path);
}
return data.map((item, i) => dehydrate(item, cleaned, path.concat([i]), level + 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

this would also be nice to limit to e.g. 100, but I'm fine w/ just putting a TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a fixed limit seems like a reasonable (but temporary) compromise.

I think the more ideal solution would be to add an optional breadth argument (defaulting to some small number (20-30) and then adding a show more... link/button in the interface which would increase the breadth and inspect+render the next 'page' of items (regardless of whether the inspected object is a iterable, array, map, etc).

I'm happy to update the PR with a hardcoded limit and add a TODO


case 'typed_array':
case 'iterator':
return createDehydrated(type, data, cleaned, path);

case 'object':
if (level > 2 || (data.constructor && typeof data.constructor === 'function' && data.constructor.name !== 'Object')) {
return createDehydrated(type, data, cleaned, path);
} else {
var res = {};
for (var name in data) {
res[name] = dehydrate(data[name], cleaned, path.concat([name]), level + 1);
}
return res;
}

default:
return data;
}
return res;
}

module.exports = dehydrate;
33 changes: 33 additions & 0 deletions agent/getIn.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
*/

/**
* Retrieves the value from the path of nested objects
* @param {Object} base Base or root object for path
* @param {Array<String>} path nested path
* @return {any} Value at end of path or `mull`
*/
function getIn(base, path) {
return path.reduce((obj, attr) => {
if (obj) {
if (obj.hasOwnProperty(attr)) {
return obj[attr];
}
if (typeof obj[Symbol.iterator] === 'function') {
// Convert iterable to array and return array[index]
return [...obj][attr];
Copy link

Choose a reason for hiding this comment

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

  1. Won't there be n^2 complexity? getIn seems to be called in a loop, and we traversing the container every time it's called.
  2. Will line 25 work with key-value maps, such as decorated records creating properties accessors for its members (so check in line Components displayed as <Unknown ... /> when declaring js variables at the top. #20 will fail)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. getIn was duplicated code moved fro Bridge.js#441 and Agent.js#401 and updated to support iterators. The complexity of the previous implementation hasn't changed (with the exception of spreading the iterable on line 25).

  2. This does work with key-value maps (if they are iterable) with one caveat, in order to support maps where the key is not a primitive the tool must treat the [key, value] entry as an iterable as well resulting in something like this:

myMap: Map(..)
  0: [
    0: <complex key A>
    1:  "some value"
  ]
  1: [
    0: "string key B"
    1:  "some other value"
  ]
  ...

see Improvement of Keyed iterables/iterators in this comment

I hope I'm making sense here.

}
}

return null;
}, base);
}

module.exports = getIn;
15 changes: 8 additions & 7 deletions frontend/DataView/DataView.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ class DataItem extends React.Component {
render() {
var data = this.props.value;
var otype = typeof data;

var complex = true;
var preview;
if (otype === 'number' || otype === 'string' || data == null /* null or undefined */ || otype === 'boolean') {
Expand All @@ -155,10 +154,11 @@ class DataItem extends React.Component {
preview = previewComplex(data);
}

var open = this.state.open && (!data || data[consts.inspected] !== false);

var inspectable = !data || !data[consts.meta] || !data[consts.meta].uninspectable;
var open = inspectable && this.state.open && (!data || data[consts.inspected] !== false);
var opener = null;
if (complex) {

if (complex && inspectable) {
opener = (
<div
onClick={this.toggleOpen.bind(this)}
Expand All @@ -172,15 +172,16 @@ class DataItem extends React.Component {

var children = null;
if (complex && open) {
var readOnly = this.props.readOnly || (data[consts.meta] && data[consts.meta].readOnly);
// TODO path
children = (
<div style={styles.children}>
<DataView
data={this.props.value}
data={data}
path={this.props.path}
inspect={this.props.inspect}
showMenu={this.props.showMenu}
readOnly={this.props.readOnly}
readOnly={readOnly}
/>
</div>
);
Expand All @@ -197,7 +198,7 @@ class DataItem extends React.Component {
{opener}
<div
style={assign({}, styles.name, complex && styles.complexName)}
onClick={this.toggleOpen.bind(this)}
onClick={inspectable && this.toggleOpen.bind(this)}
>
{this.props.name}:
</div>
Expand Down
Loading