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

Add a helper function for http/https requests in src/display/node_stream.js #18665

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

Currently we repeat virtually the same http/https request code in two different classes in this file, which seems unnecessary and it leads to more code.

…ream.js`

Currently we repeat virtually the same http/https request code in two different classes in this file, which seems unnecessary and it leads to more code.
@nicolo-ribaudo
Copy link
Contributor

I was originally trying to replace them with fetch, but that implies replacing Node.js' stream with ReadableStream and I wasn't familiar enough with the code to do it.

@Snuffleupagus
Copy link
Collaborator Author

I was originally trying to replace them with fetch, but that implies replacing Node.js' stream with ReadableStream and I wasn't familiar enough with the code to do it.

We're already using the Fetch API when possible in Node.js, please see PR #17706, by utilizing the implementation in https://github.com/mozilla/pdf.js/blob/master/src/display/fetch_stream.js; note also #17706 (comment).

@timvandermeij timvandermeij merged commit 4a13222 into mozilla:master Sep 1, 2024
8 checks passed
@timvandermeij
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants