Add indexed and named NamedNodeMap access - #723
Open
andrewiggins wants to merge 3 commits into
Open
andrewiggins wants to merge 3 commits into
andrewiggins wants to merge 3 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add live browser-style property reads to
Element.attributeswithout makingNamedNodeMapinherit fromArrayor proxying each instance:"0"through"4294967294"), rejecting fractions, aliases, and out-of-range values before linked-list index lookup.@remote-dom/polyfill.This is the focused property-access portion of #693, rebuilt on current
main. Removal/replacement fixes and attached-Attr adoption are not included.Design and performance
Fresh measurements on 2026-09-17 replace the historical benchmark claims from #693. The original harness/raw samples were unavailable, so these are reconstructed microbenchmarks, not an exact reproduction of that experiment.
Environment: Apple M4 Pro, arm64 macOS 26.6.2; Node 24.19.0 / V8 13.6.233.17-node.51; pnpm 10.28.0. These are Node/V8 measurements, not browser or application-level performance claims.
Compared exact base
a13aa618182d5c84d1c733977d1ac66955b95dbcwith PR head2f9c14177749f30b3f18f82ad85c162c3d55a9b3, built with the same pinned toolchain. A third, scratch variant applies the rejected constructor-returned per-instance proxy design to the current head while retaining its mutation logic and strict parser; it does not compare against the older branch's unrelated code.Runtime
Five fresh processes per variant/workload group, run serially in rotating variant order. Each process used 15 warm-up and 15 measured batches calibrated to roughly 10–15 ms, with observable checksums. Established-operation fixtures rotate through 257 elements with four attributes. Times include fixture selection/checksum overhead. Reported ratios are the median paired head/base time ratio, with the min–max across five process pairs—not confidence intervals. Lower is faster; differences around ±5% are treated cautiously, with no aggregate workload score.
getAttributehitattributes.lengthitem()with varying indexouterHTMLserializationMost established-operation medians remain close to baseline;
getAttributewas noisy and is not claimed as an improvement. Both measured creation paths are about 10% slower, so the change is not performance-neutral across all operations. An additional independent head/base spot-check reproduced that creation regression.Compared with the current shared fallback, the scratch per-instance proxy took 1.78–8.15× as long for the tested get/set/remove/length/item/iteration workloads, 1.08× for serialization, and 1.37× for creation + first attribute. It was not slower everywhere: indexed property reads were about 9% faster, while named hits were about 11% slower. This supports choosing the shared fallback for existing method paths rather than claiming universal superiority.
Heap
Measured post-GC
process.memoryUsage().heapUsedslopes over 0, 25,000, 50,000, and 100,000 retained elements, each with a materialized map. Five fresh processes per variant/shape; five full GCs before each reading; equal preallocated reference arrays and one separately retained Window. These are fitted used-heap bytes per element + map (+ attributes), not exact object retained sizes or RSS; a fixed shared-proxy allocation is not a per-map cost.No per-map slope increase was measured for the shared fallback versus base. The per-instance proxy adds ~32 B/map in both shapes, reproducing the earlier heap finding on the current code.
Index parser
A separate five-round property-read comparison tested the current parser against three semantically equivalent variants: bounds-first checks, unsigned-bitwise validation, and canonical-string equality before integer/bounds checks. Actual built parser exports passed accepted/rejected input checks, including symbols, fractions, aliases, numeric inputs, and array-index boundaries.
Bounds-first and bitwise variants were within ±5% of the current parser by median across the four workloads, with overlapping/noisy ranges. String-first matched indexed reads, but took 1.04× as long for named hits, 1.06× for missing names, and 1.20× for rejected numeric-looking keys (five-pair range 1.17–1.23× for the latter). This supports rejecting invalid numeric forms before string coercion; it does not establish that the current expression is universally fastest.
Bundle size
Fresh
pnpm size --jsonruns after isolated frozen-lockfile installs measured the repository's polyfill Brotli fixture at 10,893 B on base → 11,060 B on head: +167 B (+1.53%). Both pass the unchanged 12,500 B limit. These replace the old branch's inapplicable size delta.Web Platform Tests
dom/nodes/attributes-namednodemap.html.Document.createAttribute(), with one also requiring an exposedNamedNodeMapconstructor.Document-getElementById.htmlcase that mutates an ID throughelement.attributes[0].value.Intentional scope
The shared prototype fallback supports ordinary indexed and named reads, not a complete Web IDL legacy-platform-object implementation. It does not add
has, own-key/descriptor reflection, assignment/deletion restrictions, alternateReflect.get()receiver behavior, or indexed precedence over properties placed directly on a map or onNamedNodeMap.prototype.Document.createAttribute()and globalNamedNodeMapexposure remain outside this PR. Existing namespace normalization, HTML case-folding, mutation invariants, and reactions onmainare preserved rather than reimplemented.Validation
pnpm type-check,pnpm lint,pnpm build,pnpm size, andgit diff --check: passed.