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

Not working in IE11 #415

Open
omelhoro opened this issue Feb 17, 2019 · 9 comments · May be fixed by github/fetch#736
Open

Not working in IE11 #415

omelhoro opened this issue Feb 17, 2019 · 9 comments · May be fixed by github/fetch#736

Comments

@omelhoro
Copy link

@omelhoro omelhoro commented Feb 17, 2019

When I'm importing fetch-mock (7.3.0) in IE11 I get an error Expected identifier which traces to the line function findStatus(val, { useSTD3ASCIIRules }) which seems to come from 'tr46' dependency.

Since this module is imported as a string in the 'es5/client-bundle.js', webpack can't transpile it correctly (and argument destructuring actually is also not an es5 feature at all).

@wheresrhys

This comment has been minimized.

Copy link
Owner

@wheresrhys wheresrhys commented Feb 27, 2019

It'll be a while til I can look at this as will need to set up an ie11 environment and configure CI to run in ie11 too, which is not a trivial task. Would be great if you're able to work on a fix and submit a PR

@larsthorup

This comment has been minimized.

Copy link

@larsthorup larsthorup commented Mar 3, 2019

I have been looking into this. On a project we're running our unit test suite in IE11 (and other browsers) and have not been able to upgrade to fetch-mock@7 for this and a variety of other reasons.

Today I was able to get it to work with these measures:

  • require('proxy-polyfill')
  • require('unorm')
  • transpiling with babel the following node_modules dependencies required directly or indirectly by fetch-mock@7 or isomorphic-fetch:
    • proxy-polyfill
    • tr46
    • webidl-conversions
    • whatwg-url
  • applying this patch to fetch-mock: triggerz@67424f3

I would like to submit the patch as a pull request, if there is a chance that it would get merged?

@wheresrhys

This comment has been minimized.

Copy link
Owner

@wheresrhys wheresrhys commented Mar 3, 2019

Thanks Lars. I have a few questions

if ('_bodyInit' in response) {

Is this duck-typing to detect whatwg-fetch?

			response.bodyUsed = false;

Why was bodyUsed ever true here? Feels a bit weird to have to do this step and, as I don't test in ie11, a bit brittle to have the fix living in this library

How would you feel if I exposed the response constructor class, then you could do

const originalBOR = ResponseBuilder.prototype.buildObservableResponse

ResponseBuilder.prototype.buildObservableResponse = function (response) {
		if ('_bodyInit' in response) {
			// To support IE using proxy-polyfill and whatwg-fetch
			// ensure bodyUsed (of whatwg-fetch) to be properly initialized
			response.bodyUsed = false;
		}
                 return originalBOR.apply(this, response);
}

Could even publish your entire ie11 solution as an npm module in its own right

@larsthorup

This comment has been minimized.

Copy link

@larsthorup larsthorup commented Mar 4, 2019

Hey Rhys, thank you for your quick reply!

Is this duck-typing to detect whatwg-fetch?

Yes, that's correct.

Why was bodyUsed ever true here?

It was not true. The problem is that the bodyUsed property is initially missing from the response object. When whatwg-fetch then later tries to set body.bodyUsed = true (https://github.com/github/fetch/blob/master/fetch.js#L160) through the Proxy it fails with the error "Cannot create property for a non-extensible object" because the Proxy'ed response object is freezed by now (https://github.com/GoogleChrome/proxy-polyfill/blob/master/README.md)

Feels a bit weird to have to do this step

Yes, I agree. Maybe the root cause here is that the Response object is not constructed correctly by fetch-mock (https://github.com/wheresrhys/fetch-mock/blob/master/src/lib/response-builder.js#L19)? Usually when using whatwg-fetch, bodyUsed is initialized to true (https://github.com/github/fetch/blob/master/fetch.js#L209) maybe ensured by prototypical inheritance of Response from Body (https://github.com/github/fetch/blob/master/fetch.js#L400) - this style of code is not very clear to me.

If we could initialize the Response object "more correctly" - that would be a better solution. Do you have any suggestions for how to do that?

@mkay581

This comment has been minimized.

Copy link

@mkay581 mkay581 commented Sep 14, 2019

FYI I am running into the same original issue. Downgrading to fetch-mock 6.5.2 seems to fix the issue for me.

@sullivanpt

This comment has been minimized.

Copy link

@sullivanpt sullivanpt commented Nov 6, 2019

I'm unsure what solution was decided on here? ResponseBuilder.prototype.buildObservableResponse is not exposed. whatwg-fetch still has the weird pattern that doesn't include bodyUsed when constructed with new. Is there something clever you all are doing with FetchMock.config.Request to work around this? I'm trying but so far I haven't hit on the right work-around. Thanks.

Minor update: figured out that whatwg-fetch initially sets Request.prototype.bodyUsed = false which is why it isn't available to proxy-pollyfill at time of Proxy creation.

Update: I created a PR in fetch-mock to try and address this; I'm not sure they'll accept it because this Proxy use case is edgy. Will keep our fingers crossed.

For the record my use case is phantomjs-prebuilt@2.1.7, fetch-mock@8.0.0-alpha.14, proxy-pollyfill@0.3.0 on CentOS 6.

sullivanpt added a commit to sullivanpt/fetch that referenced this issue Nov 6, 2019
Fixes wheresrhys/fetch-mock#415

Details: fetch-mock wraps the Response object in an ES6 Proxy to
provide useful test harness features such as flush.  However, on
ES5 browsers without fetch or Proxy support pollyfills must be used;
the proxy-pollyfill is unable to proxy an attribute unless it exists
on the object before the Proxy is created.  This change ensures
Response.bodyUsed exists on the instance, while maintaining the
semantic of setting Request.bodyUsed in the constructor before
_initBody is called.
@sullivanpt sullivanpt linked a pull request that will close this issue Nov 6, 2019
@wheresrhys

This comment has been minimized.

Copy link
Owner

@wheresrhys wheresrhys commented Nov 11, 2019

Thanks for submitting the PR to whatwg-fetch. Hope it gets accepted.

If it's not accepted I'll consider a PR to fix here by always setting to false on setup. There's a new major version of fetch-mock coming out soon, which gives me an opportunity to fix undocumented edge cases that people may, inadvertently, be relying on being buggy

@andrewmclagan

This comment has been minimized.

Copy link

@andrewmclagan andrewmclagan commented Nov 28, 2019

Great news. started to encounter the same issues on IE-11. As a side note we are also experiencing
Edge 17+ issues:

Edge 17.17134.0 (Windows 10.0.0) ERROR
  Expected ':'
  at webpack:///node_modules/fetch-mock/esm/client.mjs:2616:31 <- karma.tests.js:46306:31

We use it to test https://github.com/wp-headless/fetch if your interested in our test harness, happy to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.