Skip to content

Proof-of-concept, relative react IDs #1547

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

Closed
wants to merge 1 commit into from
Closed

Proof-of-concept, relative react IDs #1547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 10 additions & 7 deletions perf/lib/BrowserPerfRunnerApp.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var BrowserPerfRunnerApp = React.createClass({

handleComplete: function(queueItem){
queueItem.completed = true;

if (!this.props.onCompleteEach) {
return;
}
Expand All @@ -53,7 +53,7 @@ var BrowserPerfRunnerApp = React.createClass({
.map(function(key){return this.state.results[key];}, this)
;
this.props.onCompleteEach(resultsForAllVersions);

if (this.props.onComplete && incompleteCount === 0) {
this.props.onComplete(this.state.results);
}
Expand Down Expand Up @@ -99,16 +99,16 @@ BrowserPerfRunnerApp.renderBenchmarkCell = function(props, row, col){
return benchmark && !benchmark.isRunning && benchmark.stats;
})
;

if (col == null) return React.DOM.th({style:{verticalAlign:'top', textAlign:'right'}},
React.DOM.a({href:'?test=' + row}, benchmarks[0] && benchmarks[0].name || row)
);

var key = row + '@' + col;
var benchmark = props.value[key];
if (!(benchmark && benchmark.stats)) return React.DOM.td({key:key});


var colors = [
'000000',
'AA0000',
Expand Down Expand Up @@ -197,8 +197,11 @@ var GridViewTable = React.createClass({

render: function(){
return React.DOM.table(null,
this._renderRow(null, 0),
this.props.rows.map(this._renderRow, this)
React.DOM.tbody(
null,
this._renderRow(null, 0),
this.props.rows.map(this._renderRow, this)
)
);
}

Expand Down
32 changes: 17 additions & 15 deletions perf/lib/perf-test-runner.browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ perfRunner.WriteReactLibScript = function(params){
} else {
minSuffix = '.min';
}

if (params.version && typeof params.version != 'string') throw TypeError("Expected 'version' to be a string");

if (params.version == 'edge' || !params.version) {
console.log('React edge (local)');
perfRunner.WriteScript({src:'../build/react' + minSuffix + '.js'});
Expand Down Expand Up @@ -83,7 +83,7 @@ perfRunner.getQueryParamArray = function(key){
var values;
var queryString = location.search.substr(1);
var _key = encodeURIComponent(key) + '=';

if (queryString.indexOf(_key) > -1) {
values = queryString
.split(_key)
Expand All @@ -98,7 +98,7 @@ perfRunner.getQueryParamArray = function(key){
})
;
}

perfRunner.assert(values && values.length && values[0], 'expected ' + key + ' query param');
return values;
}
Expand Down Expand Up @@ -142,7 +142,7 @@ perfRunner.quickBench = function(benchmarkOptions, onComplete, onBeforeStart){
bench.on('cycle', function(){
var bench = this,
size = bench.stats.size;

if (!bench.aborted) {
console.warn(bench.name + ' × ' + bench.count +
' (' + bench.stats.sample.length + ' samples)' +
Expand All @@ -162,12 +162,12 @@ perfRunner.quickBench = function(benchmarkOptions, onComplete, onBeforeStart){
// times: bench.times,
// stats: bench.stats
};

results['s/op'] = bench.stats.mean
results['ms/op'] = results['s/op'] * 1000
results['op/s'] = 1 / results['s/op']
results["% frame 60"] = results['ms/op'] / (1000 / 60) * 100

console.log(results);
onComplete(null, results);
});
Expand All @@ -192,13 +192,15 @@ perfRunner.singleTest = function(benchmarkOptions, onComplete){
perfRunner.ViewObject = function(props){
var value = props.value;
delete props.value;

if (typeof value != 'object') return React.DOM.span(props, [JSON.stringify(value), " ", typeof value]);

return React.DOM.table(props, Object.keys(value).map(function(key){
return React.DOM.tr(null,
React.DOM.th(null, key),
React.DOM.td(null, perfRunner.ViewObject({key:key, value:value[key]}))
);
}));

return React.DOM.table(props,
React.DOM.tbody(null, Object.keys(value).map(function(key){
return React.DOM.tr(null,
React.DOM.th(null, key),
React.DOM.td(null, perfRunner.ViewObject({key:key, value:value[key]}))
)
}))
);
}
119 changes: 88 additions & 31 deletions src/browser/ui/ReactMount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,53 @@ function getID(node) {
return id;
}

function idToNodeID(id) {
return id.substr(id.lastIndexOf('.') + 1);
}

function getNodeID(node, ancestorID) {
var id = node && node.getAttribute && node.getAttribute(ATTR_NAME);
id = id != null ? ancestorID + '.' + id : '';

if (id) {
if (nodeCache.hasOwnProperty(id)) {
var cached = nodeCache[id];
if (cached !== node) {
invariant(
!isValid(cached, id),
'ReactMount: Two valid but unequal nodes with the same `%s`: %s',
ATTR_NAME, id
);

nodeCache[id] = node;
}
} else {
nodeCache[id] = node;
}
}

return id;
}

function internalGetID(node) {
// If node is something like a window, document, or text node, none of
// which support attributes or a .getAttribute method, gracefully return
// the empty string, as if the attribute were missing.
return node && node.getAttribute && node.getAttribute(ATTR_NAME) || '';
var id = '';
while (node && node.getAttribute) {
var value = node.getAttribute(ATTR_NAME);
if (value == null) {
break;
}
id = '.' + value + id;
if (value[0] === '#') {
break;
}
node = node.parentNode;
}
return id;

//return node && node.getAttribute && node.getAttribute(ATTR_NAME) || '';
}

/**
Expand All @@ -112,7 +154,7 @@ function setID(node, id) {
if (oldID !== id) {
delete nodeCache[oldID];
}
node.setAttribute(ATTR_NAME, id);
node.setAttribute(ATTR_NAME, idToNodeID(id));
nodeCache[id] = node;
}

Expand Down Expand Up @@ -142,11 +184,13 @@ function getNode(id) {
*/
function isValid(node, id) {
if (node) {
invariant(
internalGetID(node) === id,
'ReactMount: Unexpected modification of `%s`',
ATTR_NAME
);
if (__DEV__) {
invariant(
internalGetID(node) === id,
'ReactMount: Unexpected modification of `%s`',
ATTR_NAME
);
}

var container = ReactMount.findReactContainerForID(id);
if (container && containsNode(container, node)) {
Expand All @@ -166,11 +210,11 @@ function purgeID(id) {
delete nodeCache[id];
}

var deepestNodeSoFar = null;
function findDeepestCachedAncestorImpl(ancestorID) {
var deepestNodeIDSoFar = null;
function findDeepestCachedAncestorIDImpl(ancestorID) {
var ancestor = nodeCache[ancestorID];
if (ancestor && isValid(ancestor, ancestorID)) {
deepestNodeSoFar = ancestor;
deepestNodeIDSoFar = ancestorID;
} else {
// This node isn't populated in the cache, so presumably none of its
// descendants are. Break out of the loop.
Expand All @@ -181,16 +225,16 @@ function findDeepestCachedAncestorImpl(ancestorID) {
/**
* Return the deepest cached node whose ID is a prefix of `targetID`.
*/
function findDeepestCachedAncestor(targetID) {
deepestNodeSoFar = null;
function findDeepestCachedAncestorID(targetID) {
deepestNodeIDSoFar = null;
ReactInstanceHandles.traverseAncestors(
targetID,
findDeepestCachedAncestorImpl
findDeepestCachedAncestorIDImpl
);

var foundNode = deepestNodeSoFar;
deepestNodeSoFar = null;
return foundNode;
var foundNodeID = deepestNodeIDSoFar;
deepestNodeIDSoFar = null;
return foundNodeID;
}

/**
Expand Down Expand Up @@ -549,8 +593,7 @@ var ReactMount = {
// Not a DOMElement, therefore not a React component
return false;
}
var id = ReactMount.getID(node);
return id ? id.charAt(0) === SEPARATOR : false;
return node.hasAttribute && node.hasAttribute(ATTR_NAME);
},

/**
Expand Down Expand Up @@ -586,32 +629,51 @@ var ReactMount = {
var firstChildren = findComponentRootReusableArray;
var childIndex = 0;

var deepestAncestor = findDeepestCachedAncestor(targetID) || ancestorNode;
var deepestAncestorID = findDeepestCachedAncestorID(targetID);
var deepestAncestor;
if (deepestAncestorID) {
deepestAncestor = ReactMount.getNode(deepestAncestorID);
} else {
deepestAncestorID = ReactMount.getID(ancestorNode);
deepestAncestor = ancestorNode;
}

firstChildren[0] = deepestAncestor.firstChild;
firstChildren.length = 1;

var ancestorID = deepestAncestorID;

while (childIndex < firstChildren.length) {
var child = firstChildren[childIndex++];
var targetChild;

while (child) {
var childID = ReactMount.getID(child);
var childID = ReactMount.getNodeID(child, ancestorID);

if (childID) {
// Even if we find the node we're looking for, we finish looping
// through its siblings to ensure they're cached so that we don't have
// to revisit this node again. Otherwise, we make n^2 calls to getID
// when visiting the many children of a single node in order.

//console.log(targetID, childID);
if (targetID === childID) {
targetChild = child;
var targetChild = child;

// Emptying firstChildren/findComponentRootReusableArray is
// not necessary for correctness, but it helps the GC reclaim
// any nodes that were left at the end of the search.
firstChildren.length = 0;

return targetChild;
} else if (ReactInstanceHandles.isAncestorIDOf(childID, targetID)) {
// If we find a child whose ID is an ancestor of the given ID,
// then we can be sure that we only want to search the subtree
// rooted at this child, so we can throw out the rest of the
// search state.
firstChildren.length = childIndex = 0;
firstChildren.push(child.firstChild);
ancestorID = childID;
break;
}

} else {
Expand All @@ -625,19 +687,12 @@ var ReactMount = {

child = child.nextSibling;
}

if (targetChild) {
// Emptying firstChildren/findComponentRootReusableArray is
// not necessary for correctness, but it helps the GC reclaim
// any nodes that were left at the end of the search.
firstChildren.length = 0;

return targetChild;
}
}

firstChildren.length = 0;

//console.log(ancestorID);

invariant(
false,
'findComponentRoot(..., %s): Unable to find element. This probably ' +
Expand All @@ -659,6 +714,8 @@ var ReactMount = {

getID: getID,

getNodeID: getNodeID,

setID: setID,

getNode: getNode,
Expand Down
2 changes: 1 addition & 1 deletion src/browser/ui/dom/DOMPropertyOperations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var DOMPropertyOperations = {
*/
createMarkupForID: function(id) {
return processAttributeNameAndPrefix(DOMProperty.ID_ATTRIBUTE_NAME) +
escapeTextForBrowser(id) + '"';
escapeTextForBrowser(id.substr(id.lastIndexOf('.') + 1)) + '"';
},

/**
Expand Down
2 changes: 1 addition & 1 deletion src/core/ReactInstanceHandles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ var ReactInstanceHandles = {
* @return {string} A React root ID.
*/
createReactRootID: function() {
return getReactRootIDString(ReactRootIndex.createReactRootIndex());
return getReactRootIDString('#' + ReactRootIndex.createReactRootIndex());
},

/**
Expand Down