Skip to content

Migrate node-fetch to built-in fetch (undici) - #5487

Merged
boutell merged 4 commits into
mainfrom
PRO-9597-fetch
Jun 24, 2026
Merged

boutell merged 4 commits into
mainfrom
PRO-9597-fetch

Conversation

@myovchev

Copy link
Copy Markdown
Contributor

Please indicate which branch this PR should merge into:

  • main
  • latest
  • stable

Summary

Migrate node-fetch to built-in fetch (undici), remove the node-fetch dep. Full details in the changelog.
This is required due to the fact node-fetch is a deprecated and not following node standards. A better long term contract.

What are the specific steps to test this change?

No internal BC break.

What kind of change does this PR introduce?

(Check at least one)

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Build-related changes
  • Other

Make sure the PR fulfills these requirements:

  • It includes a) the existing issue ID being resolved, b) a convincing reason for adding this feature, or c) a clear description of the bug it resolves
  • The changelog is updated
  • Related documentation has been updated
  • Related tests have been updated

If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@linear

linear Bot commented Jun 23, 2026

Copy link
Copy Markdown

PRO-9597

@myovchev
myovchev requested a review from boutell June 23, 2026 14:01

@boutell boutell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also note tests are failing.

"apostrophe": minor
---

The server-side HTTP client (`apos.http`) now uses Node's built-in `fetch` instead of `node-fetch`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to provide a justification for why this is not a major version break. I think because node-fetch is no longer maintained?

@boutell

boutell commented Jun 23, 2026 via email

Copy link
Copy Markdown
Member

@boutell

boutell commented Jun 23, 2026 via email

Copy link
Copy Markdown
Member

@myovchev

myovchev commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Changelog now clarifies the reasons and BC state of things, should be much clear now.
Tests fixed (an IPv6 CI gotcha).
There is also a bonus fix - a flaky test that hit my CI run and revealed a true racing condition in the Batch module.

@myovchev
myovchev requested a review from boutell June 24, 2026 09:10
self.setTotal(job, ids.length);
// Persist the total before work begins so the completed notification
// and job document always report it, even if processing finishes fast.
await self.setTotal(job, ids.length);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A bonus fix, unrelated to the ticket scope, but a real world problem revealed by randomly failing test.

@boutell
boutell merged commit 68f1312 into main Jun 24, 2026
27 checks passed
@boutell
boutell deleted the PRO-9597-fetch branch June 24, 2026 16:59
myovchev added a commit that referenced this pull request Jun 24, 2026
* Migrate node-fetch to built-in fetch (undici), remove the node-fetch dep

* Better changelog details, fix IPv6 test issues (CI)

* Fix batch total race
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.

2 participants