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

Switch to fetch for url requests #1165

Merged
merged 4 commits into from
Feb 6, 2023
Merged

Switch to fetch for url requests #1165

merged 4 commits into from
Feb 6, 2023

Conversation

danielholmes
Copy link
Collaborator

@danielholmes danielholmes commented Feb 6, 2023

What's Changing and Why

Core/request is the last source file using commonJS exports. It's not a simple syntax change though since it uses a feature of CJS not available in ESM - conditional exports. Specifically, if in a browser or electron environment then the global XMLHttpRequest is being used, but in a node environment the node only package phin is being used.

This is one of the hard blockers for ESM based tool compatibility (e.g. vite and rollup). See my example repo for a playground trying to get this working.

NodeJS has fetch built in from v18 onwards and browser compatibility for fetch is now very high. I propose a move to fetch instead of the conditional requires.

I went back and forth about whether to use fetch and rely on the library users to install a polyfill if needed or to include a polyfill within jimp. Without the polyfill I think a lot of projects will break and I think that should maybe be done in a "big" release (like 1.0), but let me know what you think.

What else might be affected

loadFromURL can take a set of options. When in the node environment all those options are passed to phin. Any options used to pass to phin that are not supported by the fetch API will stop working.

Tasks

  • Add tests
  • Update Documentation
  • Update jimp.d.ts
  • Add SemVer Label

Release Notes

The underlying library for fetching images has been changed.

loadFromURL can take a set of options. When in the node environment all those options are passed to phin. Any options used to pass to phin that are not supported by the fetch API will stop working.

@hipstersmoothie hipstersmoothie merged commit b3b6438 into jimp-dev:master Feb 6, 2023
@hipstersmoothie hipstersmoothie added the major Increment the major version when merged label Feb 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

🚀 PR was released in v0.22.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Increment the major version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants