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

The preferNative option doesn't work #321

Open
kanongil opened this issue Jun 18, 2019 · 5 comments
Open

The preferNative option doesn't work #321

kanongil opened this issue Jun 18, 2019 · 5 comments

Comments

@kanongil
Copy link

When I set preferNative: true, in a build that includes the polyfill, fetch always uses the polyfill code.

@sandstrom
Copy link

sandstrom commented Aug 28, 2019

Two thoughts:

  1. Somewhat related, it would be great if the readme would clarify what purpose this lib serves when native is already available. I guess the answer is the run-loop wrapping, if so would be great to include a small snippet on how to handle that manually, without this library.

  2. Perhaps this lib could read the data in targets.js, to determine if the polyfill is needed.

@rahulk94
Copy link

rahulk94 commented Sep 8, 2021

Any updates on this? Just tried using the preferNative config option and still see it being polyfilled in browsers that don't need it (e.g. Chrome 92). I do have IE11 in my supported browsers list but I would've thought the expected behaviour would be to use the native code if available.

Fwiw I've tried both v7.0.0 and v8.0.1.

@nlfurniss
Copy link
Collaborator

Any updates on this? Just tried using the preferNative config option and still see it being polyfilled in browsers that don't need it (e.g. Chrome 92). I do have IE11 in my supported browsers list but I would've thought the expected behaviour would be to use the native code if available.

Fwiw I've tried both v7.0.0 and v8.0.1.

If all your browser targets support native fetch, and preferNative: true, the polyfill will not be included in the output build.

Your build doesn’t meet the criteria for the polyfill to not be included

@rahulk94
Copy link

rahulk94 commented Sep 8, 2021

Hmmm the docs feel a bit misleading then?

If set to true, the fetch polyfill will be skipped if native fetch is available, otherwise the polyfilled fetch will be installed during the first pass of the vendor js file.
If set to false, the polyfilled fetch will replace native fetch be there or not.

Makes it sound like this config option determines whether the polyfill should early exit and not be used if the browser already supports it.

If all your browser targets support native fetch, and preferNative: true, the polyfill will not be included in the output build. If, for some reason, you still need the polyfill to be included in the bundle, you can set alwaysIncludePolyfill: true.

Feels more like documentation around when the polyfill will be included in your bundle, not whether it's applied though.

Anywho thanks for confirming this @nlfurniss.

@nlfurniss
Copy link
Collaborator

Hmmm the docs feel a bit misleading then?

If set to true, the fetch polyfill will be skipped if native fetch is available, otherwise the polyfilled fetch will be installed during the first pass of the vendor js file.
If set to false, the polyfilled fetch will replace native fetch be there or not.

Makes it sound like this config option determines whether the polyfill should early exit and not be used if the browser already supports it.

If all your browser targets support native fetch, and preferNative: true, the polyfill will not be included in the output build. If, for some reason, you still need the polyfill to be included in the bundle, you can set alwaysIncludePolyfill: true.

Feels more like documentation around when the polyfill will be included in your bundle, not whether it's applied though.

Anywho thanks for confirming this @nlfurniss.

Yup, totally see how this is not 100% clear. I'll put up a PR to clarify that the polyfill is included if any of the babel targets require fetch, and that it isn't impacted by which UA is loading the app/addon

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

No branches or pull requests

4 participants