Repository navigation
Track requests without replacing XMLHttpRequest - #571
Merged
Merged
Conversation
LICENSE keeps HubSpot's notice and adds one for Zachary Miller, who has maintained pace since 2019. The headers of pace.js and pace-theme-default.css name both, package.json's author field uses npm's "name <email> (url)" form, and the README credit shows the name.
Add the current maintainer's copyright notice
Brings in the current maintainer's copyright notice. pace.min.js is rebuilt from the merged pace.js.
Pace used to replace window.XMLHttpRequest with a function returning native
objects, and WebSocket the same way. Pages then got a different global:
instanceof checks failed, copying or enumerating the constructor's
properties (Object.assign({}, XMLHttpRequest)) threw, subclasses lost their
methods, and libraries that patch XMLHttpRequest.prototype after pace
(Sentry, zone.js) never saw requests, because pace gave every request its
own open(). The wrapper was installed even with
`ajax: false`. The idea of patching the prototype instead comes from
a2intl's work on sharing XMLHttpRequest's prototype.
Now:
- Pace wraps XMLHttpRequest.prototype.open and send, and replaces
WebSocket with a subclass of the native one, so the page keeps the native
classes. Nothing is installed until a run first tracks ajax, and nothing
at all with `ajax: false`, where Pace.track no longer throws either.
- A request is tracked from send() until it ends, instead of from open()
until load, abort, timeout or error. A request that was opened but never
sent, aborted before sending, or whose open() threw no longer holds the
bar forever. Opening a request again ends the one it replaces, and a
request sent again from its own load handler is tracked until the second
one ends.
- When a run is stopped or restarted (by a navigation, say), the next run
keeps tracking the requests it was tracking that are still loading.
Before, they were dropped and the new run could finish early.
- ajax.ignoreURLs is checked once per request: a URL object no longer
throws, a RegExp with the g flag no longer matches only every other time,
and entries that are neither strings nor RegExps are skipped.
The README notes that requests sent before Pace first starts aren't
tracked, and that Pace should load before a library that replaces
window.XMLHttpRequest.
The test sent its requests during page load. In WebKit under Playwright's request routing, document.readyState stays "interactive" until those requests finish, so the document collector held the bar even though the requests were correctly ignored. On an idle page, two matching requests must not restart the bar; the test still fails without the lastIndex reset in all three engines.
- A request ends when its readystatechange reaches readyState 4, or 0 when the page aborts it from that handler, instead of at a loadend with readyState 4. Every way a request ends passes one of these, and a request in flight is always at 1 to 3. Before, abort() after a request finished (from onload or onreadystatechange) left it open, so every later run waited for it. - Only Pace.restart hands a run's still-loading requests to the next run, and it gives them to whichever run is going afterwards, even when stop or start handlers replaced it. Before, Pace.stop also handed them over, so an unrelated run much later waited for requests the page had stopped showing, and a stop handler that started pace again lost them. - A run that starts while a request waits for its restartOnRequestAfter check picks it up, so a navigation right after a request (as Turbolinks does) shows one bar instead of two. - That check now asks whether the request itself is still loading, rather than reading its readyState, so a request reopened from its load handler without being sent no longer restarts the bar. - The ajax collector keeps only the trackers of requests still loading, so finished requests can be garbage collected, and end callbacks belong to a request's event rather than its XMLHttpRequest. Tests cover each case; the ones that depend on a request's end use a real local server with responses of unknown length, because with a known length the request's progress reaches 100% and hides a request that never ends.
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.
Pace no longer replaces
window.XMLHttpRequestandWebSocket. It wrapsXMLHttpRequest.prototype.openandsendand subclassesWebSocket, so pages keep the native classes:instanceof, constants, subclasses and other libraries' prototype patches work again. The commit message lists each change; in short:ajax: false(wherePace.trackno longer throws).send()until it ends, so requests that are never sent, aborted early, or whoseopen()throws can't hold the bar. Reused XMLHttpRequests are tracked per request.ajax.ignoreURLshandlesURLobjects, RegExps with thegflag, and invalid entries.Thanks to @a2intl, whose work on sharing XMLHttpRequest's prototype pointed the way to patching it directly.
Also brings in
master(the current maintainer's copyright notice).Tests:
tests/requests.spec.mjs. Each new test fails on the previous code, except a few guards for behavior that stays the same (a WebSocket holding the bar, tested against a real local server).