Skip to content
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.

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

attach onerror callback to the image used for ajax#getImage #5240

Closed
wants to merge 1 commit into from

Conversation

kturney
Copy link

@kturney kturney commented Sep 5, 2017

I ran into there being no error callback while trying to map#loadImage a url to an svg instead of a png.

The error message could probably be better, and I'm not sure how y'all feel about attaching the original error as the domErr property.

The getImage, invalid image test is currently failing with

Error: Fake server request processing threw exception: URL.createObjectURL is not a function
      at TypeError: URL.createObjectURL is not a function
      at     at exports.getArrayBuffer (src/util/ajax.js:136:53)
      at     at FakeXMLHttpRequest.xhr.onload (src/util/ajax.js:94:13)
      at     at Array.forEach (<anonymous>)
      at     at Object.window.server.respondWith.request [as response] (test/unit/util/ajax.test.js:100:21)
      at     at Array.forEach (<anonymous>)
      at     at Test.t.test (test/unit/util/ajax.test.js:107:23)

so I'm not sure why that wouldn't be there.

@jfirebaugh
Copy link
Contributor

We use jsdom in the tests, and it doesn't implement URL.createObjectURL or URL.revokeObjectURL. Unless there's some creative way to bypass those with mocks, we might not be able to test this currently.

@kturney
Copy link
Author

kturney commented Sep 18, 2017

So what is the desired solution?

Delete the tests or (somewhat hackily) stub out URL.createObjectURL and URL.revokeObjectURL if they do not exist?

I can implement whichever you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants