Skip to content

fix: build the id index only when an element is looked up by id #52

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

Merged
alan-agius4 merged 1 commit into from
Sep 24, 2026
Merged

fix: build the id index only when an element is looked up by id #52

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
102 changes: 72 additions & 30 deletions lib/Document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,18 @@ function Document(isHTML, address) {
// We may need to update this mapping every time a node is rooted
// or uprooted, and any time an attribute is added, removed or changed
// on a rooted element.
this.byId = Object.create(null);
//
// The mapping is built lazily: it stays null until something actually
// looks an element up by id, and only then is it populated from the
// nodes currently rooted in this document. After that it is kept up to
// date by root()/uproot() and by the id attribute change handler, just as
// it always was. Documents that are never queried by id -- in particular
// the private documents the fragment parser creates and throws away for
// every innerHTML assignment -- no longer pay for an index nobody reads.
//
// A Map keeps ids such as `1023` out of sparse object-index storage, the
// same reason Element uses one for qualified attribute names.
this._byId = null;

// This property holds a monotonically increasing value akin to
// a timestamp used to record the last modification time of nodes
Expand Down Expand Up @@ -326,7 +337,9 @@ Document.prototype = Object.create(ContainerNode.prototype, {
}},

getElementById: { value: function(id) {
var n = this.byId[id];
// Coercion can mutate the document, so do it before building the index.
id = String(id);
var n = this._idIndex().get(id);
if (!n) return null;
if (n instanceof MultiId) { // there was more than one element with this id
return n.getFirst();
Expand All @@ -336,7 +349,29 @@ Document.prototype = Object.create(ContainerNode.prototype, {

_hasMultipleElementsWithId: { value: function(id) {
// Used internally by querySelectorAll optimization
return (this.byId[id] instanceof MultiId);
return (this._idIndex().get(id) instanceof MultiId);
}},

// The id -> element map, materialized on demand. Everything that reads the
// index goes through here; everything that writes it (addId/delId) leaves it
// alone while it is absent.
_idIndex: { value: function _idIndex() {
var byId = this._byId;
if (byId !== null) return byId;
byId = this._byId = new Map();
// Populate from the elements rooted in this document right now. _nodes
// is the registry root() and uproot() maintain, so walking it -- rather
// than the tree -- gives exactly the entries the index would hold had it
// been kept up to date all along, and needs no traversal state.
var nodes = this._nodes;
for (var nid = 1, end = this._nextnid; nid < end; nid++) {
var n = nodes[nid];
if (n !== undefined && n.nodeType === Node.ELEMENT_NODE) {
var id = n.getAttributeNS(null, 'id');
if (id) this.addId(id, n);
}
}
return byId;
}},

// Just copy this method from the Element prototype
Expand Down Expand Up @@ -631,34 +666,38 @@ Document.prototype = Object.create(ContainerNode.prototype, {

// Add a mapping from id to n for n.ownerDocument
addId: { value: function addId(id, n) {
var val = this.byId[id];
var byId = this._byId;
if (byId === null) return; // no index yet; it will pick n up when built
var val = byId.get(id);
if (!val) {
this.byId[id] = n;
byId.set(id, n);
}
else {
// TODO: Add a way to opt-out console warnings
//console.warn('Duplicate element id ' + id);
if (!(val instanceof MultiId)) {
val = new MultiId(val);
this.byId[id] = val;
byId.set(id, val);
}
val.add(n);
}
}},

// Delete the mapping from id to n for n.ownerDocument
delId: { value: function delId(id, n) {
var val = this.byId[id];
var byId = this._byId;
if (byId === null) return; // no index yet; nothing to remove n from
var val = byId.get(id);
utils.assert(val);

if (val instanceof MultiId) {
val.del(n);
if (val.length === 1) { // convert back to a single node
this.byId[id] = val.downgrade();
byId.set(id, val.downgrade());
}
}
else {
this.byId[id] = undefined;
byId.delete(id);
}
}},

Expand Down Expand Up @@ -774,8 +813,10 @@ function root(n) {
n.ownerDocument._nodes[n._nid] = n;
// Manage id to element mapping
if (n.nodeType === Node.ELEMENT_NODE) {
var id = n.getAttribute('id');
if (id) n.ownerDocument.addId(id, n);
if (n.ownerDocument._byId !== null) {
var id = n.getAttributeNS(null, 'id');
if (id) n.ownerDocument.addId(id, n);
}

// Script elements need to know when they're inserted
// into the document
Expand All @@ -785,8 +826,8 @@ function root(n) {

function uproot(n) {
// Manage id to element mapping
if (n.nodeType === Node.ELEMENT_NODE) {
var id = n.getAttribute('id');
if (n.nodeType === Node.ELEMENT_NODE && n.ownerDocument._byId !== null) {
var id = n.getAttributeNS(null, 'id');
if (id) n.ownerDocument.delId(id, n);
}
n.ownerDocument._nodes[n._nid] = undefined;
Expand Down Expand Up @@ -829,27 +870,31 @@ function recursivelySetOwner(node, owner) {
recursivelySetOwner(kid, owner);
}

// A class for storing multiple nodes with the same ID
// A class for storing multiple nodes with the same ID.
//
// A Map keeps the node ids used as keys out of sparse object-index storage:
// a plain object keyed by integers gets an elements backing store sized to
// the largest key, which for node ids means one as large as the document for
// every id that is duplicated.
function MultiId(node) {
this.nodes = Object.create(null);
this.nodes[node._nid] = node;
this.nodes = new Map();
this.nodes.set(node._nid, node);
this.length = 1;
this.firstNode = undefined;
}

// Add a node to the list, with O(1) time
MultiId.prototype.add = function(node) {
if (!this.nodes[node._nid]) {
this.nodes[node._nid] = node;
if (!this.nodes.has(node._nid)) {
this.nodes.set(node._nid, node);
this.length++;
this.firstNode = undefined;
}
};

// Remove a node from the list, with O(1) time
MultiId.prototype.del = function(node) {
if (this.nodes[node._nid]) {
delete this.nodes[node._nid];
if (this.nodes.delete(node._nid)) {
this.length--;
this.firstNode = undefined;
}
Expand All @@ -861,24 +906,21 @@ MultiId.prototype.del = function(node) {
MultiId.prototype.getFirst = function() {
/* jshint bitwise: false */
if (!this.firstNode) {
var nid;
for (nid in this.nodes) {
if (this.firstNode === undefined ||
this.firstNode.compareDocumentPosition(this.nodes[nid]) & Node.DOCUMENT_POSITION_PRECEDING) {
this.firstNode = this.nodes[nid];
var self = this;
this.nodes.forEach(function(node) {
if (self.firstNode === undefined ||
self.firstNode.compareDocumentPosition(node) & Node.DOCUMENT_POSITION_PRECEDING) {
self.firstNode = node;
}
}
});
}
return this.firstNode;
};

// If there is only one node left, return it. Otherwise return "this".
MultiId.prototype.downgrade = function() {
if (this.length === 1) {
var nid;
for (nid in this.nodes) {
return this.nodes[nid];
}
return this.nodes.values().next().value;
}
return this;
};
Loading
Loading