d3 v5 #6245
Merged
d3 v5 #6245
Conversation
This file contains 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
You can only consume a response once - thereafter the response body becomes "locked" and will throw an error if you try. https://stackoverflow.com/questions/53511974/javascript-fetch-failed-to-execute-json-on-response-body-stream-is-locked
Because the done callback is expecting something that has a `status` property, (like an XHR would), we need to extract the status out of the error message. d3-xml includes status in the error message, but we can't access the response itself.
This mimics `respondWith` and `respond` to keep the tests mostly similar between `FakeFetch` and `sinon.fakeServer` Major difference is that all the fakeServer tests which were sync are now async and this is probably unavoidable.
- Many text expects are now wrapped in setTimeout, as the fetch promises settle async now. - This makes the tests somewhat brittle, and we should maybe consider reworking some of them. For example it is very hard to perform a test like `expect(spy).to.have.not.been.called` in an async way. (We could instead inspect the fakeServer requests() to know this.) - Also includes some trickery for osm.js, which uses d3-xml (fetch) now for unauthenticated calls and osmauth (xhr) for authenticated calls
This isn't perfect but might avoid spurious test failures.
This lets us call `respond` multiple times, which is useful for the tests which fetch data in pages.
- can't reliably use sinon.spy to tell whether a thing has been called, so we listen for events instead and check server.requests() - make sure to request the next page before dispatching `loadedImages` so we can `server.respond()` to the request in the event handler if we want to - also moves `localeDateString` in the openstreetcam service from parsing code to display code, because it's very slow (we can just do this for the images we look at, instead of all images we fetch)
|
Good enough for now.. |
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.
Upgrades iD to use d3 v5
The main change here is replacing the code that used d3-request to now use d3-fetch, and big updates to the test code to handle this change, such as adding a sinon.stub for the fetch API, and making most of those tests work async.