perf_hooks: add missing resource timing attributes - #65017
Conversation
|
Review requested:
|
74a9a8a to
2ebef57
Compare
Add the finalResponseHeadersStart, firstInterimResponseStart, renderBlockingStatus, contentType and contentEncoding getters to PerformanceResourceTiming and update the WPT status accordingly. Signed-off-by: greenhead <shren0812@gmail.com>
2ebef57 to
94dca40
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65017 +/- ##
==========================================
- Coverage 90.30% 90.07% -0.23%
==========================================
Files 759 754 -5
Lines 247621 256399 +8778
Branches 46672 48495 +1823
==========================================
+ Hits 223603 230948 +7345
- Misses 15473 16582 +1109
- Partials 8545 8869 +324
🚀 New features to boost your workflow:
|
|
Hello @legendecas @jasnell 🖐️ Thank you for all the work that goes into reviewing contributions here! Just a gentle ping in case this PR slipped through the cracks. I would appreciate any feedback whenever you have time, no rush at all. Thanks! |
The spec defines responseStart as firstInterimResponseStart when that is not 0, and finalResponseHeadersStart otherwise. Signed-off-by: greenhead <greenheadhq@gmail.com>
Document the five getters PerformanceResourceTiming gained: finalResponseHeadersStart, firstInterimResponseStart, renderBlockingStatus, contentType and contentEncoding, add the missing responseStart entry, and record its new interim-aware behavior. Signed-off-by: greenhead <greenheadhq@gmail.com>
| The high resolution millisecond timestamp representing the time immediately | ||
| after Node.js receives the first byte of the first interim response, such as | ||
| a `103 Early Hints` response. Node.js does not currently record interim | ||
| responses, so the property always returns 0. |
There was a problem hiding this comment.
The added test shows that this property can return a non-zero value even though interim responses are not currently recorded.
assert.strictEqual(resource.firstInterimResponseStart, 45);
This mixes the API's capabilities with current Node.js behavior. The other added properties appear to have the same issue.
There was a problem hiding this comment.
Thanks, I missed that distinction. I updated the affected docs to separate the API behavior from the values currently produced by built-in fetch().
Signed-off-by: greenhead <greenheadhq@gmail.com>
Signed-off-by: greenhead <greenheadhq@gmail.com>
Add the
PerformanceResourceTimingattributes from the Resource Timing specification that are still missing in Node.js:finalResponseHeadersStart,firstInterimResponseStart,renderBlockingStatus,contentType, andcontentEncoding.The new getters follow the existing
PerformanceResourceTimingpattern and are included intoJSON(). The WPT status file is updated to enable the tenidlharnesssubtests that now pass.Update
responseStartto returnfirstInterimResponseStartwhen it is non-zero and fall back tofinalResponseHeadersStartotherwise, matching the Resource Timing specification.Tests cover the new getters, default and supplied metadata, the
interim = 0, final > 0fallback, and an entry created by the built-infetch()implementation.Refs: #51589