Skip to content

Conversation

@krystofwoldrich
Copy link
Contributor

@krystofwoldrich krystofwoldrich commented Nov 7, 2025

Why

I was running into the following error when using the Snack SDK in CF Workers.

I've tracked down the issue to an old node-fetch.

Error on server: TypeError: Cannot read properties of null (reading 'has')
    at processHeader (node-internal:internal_http_outgoing:904:39)
    at ClientRequest._storeHeader (node-internal:internal_http_outgoing:209:21)
    at ClientRequest._implicitHeader (node-internal:internal_http_client:414:14)
    at ClientRequest.#write (node-internal:internal_http_outgoing:823:18)
    at ClientRequest.write (node-internal:internal_http_outgoing:607:32)
    at ClientRequest.write (node-internal:internal_http_client:382:22)
    at writeToStream (snack-sdk.js:10347:10)
    at snack-sdk.js:10688:5
    at new Promise (<anonymous>)
    at fetch2 (snack-sdk.js:10493:10)

How

This PR removes the current ponyfill and defaults to global fetch impl of the runtime.

Since all modern browser have a solid fetch implementation, I think we could use the global for web. Likely we could do the same for Expo Apps.

For modern node ponyfill would also not be needed. But for other envs for example CF Workers, we need the ability to change the fetch function. So I exposed a public function setSnackSDKFetch(fn).

Test Plan

@krystofwoldrich krystofwoldrich changed the title chore: replace fetch ponyfill by node fetch chore: remove fetch ponyfill Nov 13, 2025
@krystofwoldrich krystofwoldrich force-pushed the @krystofwoldrich/upgrade-fetch branch from fd4e7d4 to ac0322b Compare November 13, 2025 00:14
@krystofwoldrich krystofwoldrich marked this pull request as ready for review November 13, 2025 00:15
@krystofwoldrich krystofwoldrich changed the title chore: remove fetch ponyfill chore: remove fetch ponyfill, use global.fetch Nov 27, 2025
Copy link
Member

@byCedric byCedric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need to fix CI before merging, but it looks good overall.

@byCedric byCedric changed the title chore: remove fetch ponyfill, use global.fetch chore: replace fetch ponyfill with global fetch Nov 27, 2025
@byCedric byCedric changed the title chore: replace fetch ponyfill with global fetch refactor(snack-sdk): replace fetch ponyfill with global fetch Nov 27, 2025
@krystofwoldrich krystofwoldrich force-pushed the @krystofwoldrich/upgrade-fetch branch from b7aafe1 to 8fb3daa Compare November 27, 2025 15:31
@krystofwoldrich krystofwoldrich force-pushed the @krystofwoldrich/upgrade-fetch branch from 8fb3daa to 486bbac Compare November 27, 2025 19:09
@krystofwoldrich krystofwoldrich changed the base branch from main to @krystofwoldrich/chore/bump-jest November 27, 2025 19:10
@krystofwoldrich krystofwoldrich force-pushed the @krystofwoldrich/upgrade-fetch branch from 486bbac to 80bc5fd Compare November 27, 2025 20:14
@krystofwoldrich krystofwoldrich force-pushed the @krystofwoldrich/upgrade-fetch branch 2 times, most recently from f388fa8 to 9e4bb20 Compare November 27, 2025 20:47
Base automatically changed from @krystofwoldrich/chore/bump-jest to main December 1, 2025 03:56
@byCedric byCedric force-pushed the @krystofwoldrich/upgrade-fetch branch from 2f9e718 to fee1924 Compare December 1, 2025 04:05
@byCedric byCedric merged commit e161e0e into main Dec 1, 2025
29 checks passed
@byCedric byCedric deleted the @krystofwoldrich/upgrade-fetch branch December 1, 2025 04:17
@byCedric byCedric mentioned this pull request Dec 1, 2025
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