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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing undici dependency #2065

Merged
merged 4 commits into from May 1, 2024
Merged

Add missing undici dependency #2065

merged 4 commits into from May 1, 2024

Conversation

frandiox
Copy link
Contributor

@frandiox frandiox commented May 1, 2024

We were using undici for the devtools but forgot to add it to dependencies. It generally works because it is added by miniflare but I've seen some situations reported where it cannot find the dependency.

I've tried to remove it and use node:https instead but found issues, probably related to https://github.com/orgs/nodejs/discussions/49734

The version of undici added to our dependencies is the same Miniflare uses to avoid adding duplicates.

To 馃帺 this, simply run the skeleton with npm run dev -- --debug and open the devtools. It should work without errors.

@frandiox frandiox requested a review from a team May 1, 2024 03:42
Comment on lines +298 to 300
response.body.pipe(nodeResponse);
}

return response.body.pipe(nodeResponse);
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it matter that this doesn't return anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we were not using the returned value.

@frandiox frandiox merged commit 72e6794 into main May 1, 2024
13 checks passed
@frandiox frandiox deleted the fd-missing-undici branch May 1, 2024 23:01
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.

None yet

2 participants