-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix: fetch api polyfill #3262
fix: fetch api polyfill #3262
Conversation
🦋 Changeset detectedLatest commit: d9d68b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Deploy preview for merchant-center-application-kit ready! ✅ Preview Built with commit d9d68b0. |
@tdeekens @emmenko @abreu-ct I understand why someone would add a polyfill for fetch since that is what a quick google would suggest but that polyfill isn't needed when you use node 18 and would not be needed in a browser for quite some time. The root cause of this issue is because the jsdom environment is outdated and a custom profile representing a more up to date jsdom environment would have solved this better in my opinion. |
Thanks for the feedback. That sounds indeed like a better solution. I'll take a note for us to look into that as a follow up. |
Thanks make sense. Let's follow-up and first unblock the team :) |
This PR adds
Request
andResponse
to the Fetch API polyfill. Unfortunatelyjsdom
does not provide a way to use Node native Fetch API, I used the same approach for theHeaders
class and also exportedRequest
andResponse
fromnode-fetch
.This PR will allow running jest tests with
jsdom
env that relies on the Fetch API fixing https://github.com/commercetools/merchant-center-frontend/pull/15461Notes:
In setup-test-framework.js#L2 we are using the
unfetch/polyfill
to export the Fetch function and relying onnode-fetch
to exportHeaders
,Request
andResponse
. I tried to changeunfetch/polyfill
polyfill but unfortunately some tests inmerchant-center-frontend
started to fail.