Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

fix: patch issues with workers #282 #288

Merged
merged 2 commits into from
Mar 27, 2023
Merged

Conversation

matthias-heller
Copy link
Contributor

As described in https://github.com/vercel/pkg-fetch/issues/282 there is an issue with workers starting from node.js 18.14.1. This fix solves this issue for 18.15.0 as well as 19.8.1

This commit solves issue #282 for node.js 18.15.0
This commit solves issue #282 for node.js 19.8.1
@baparham
Copy link
Contributor

building assets here to test that these patches build in CI

@robertsLando
Copy link
Contributor

@baparham Can I merge this?

@baparham
Copy link
Contributor

builds are still running, but looking positive. I'd like to test out the binary myself (to ensure we didn't break non-worker usage) before saying yes

@robertsLando
Copy link
Contributor

ok LMK

@baparham
Copy link
Contributor

I tried running with windows x64 18.15.0 from the CI builds, but pkg was complaining it can't find the placeholder. I may have setup my test incorrectly with the wrong cache or something applied, but I won't be able to come back and try again right away, just to let you know what the status is. I'm not 100% confident in this quite yet but I appreciate your patience.

@baparham
Copy link
Contributor

giving this another look today, FYI

@baparham
Copy link
Contributor

OK, so I tested it out with a bare bones pkg hello world test for workers and main threads and it seems OK. I'm not sure what I was doing wrong in my production repo, so hopefully this doesn't break that use case 😬

@robertsLando
Copy link
Contributor

Good to know! Good to merge so? 🚀

Copy link
Contributor

@baparham baparham left a comment

Choose a reason for hiding this comment

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

yep!

@baparham baparham merged commit 0098fa8 into vercel:main Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants