Skip to content

Remove cross-fetch usage #124

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.

Sign up for GitHub

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

Jump to bottom
Open
Copilot wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Remove cross-fetch usage #124

Copilot wants to merge 2 commits into from

Conversation

Copy link

Copilot AI commented Jan 3, 2026 •
edited
Loading

fetch is a global API as of Node 18, which is now the minimum supported version. This removes the cross-fetch polyfill dependency.

Changes

  • Source files: Replace cross-fetch imports with native fetch in:

    • packages/connected-solid/src/util/guaranteeFetch.ts
    • packages/solid-react/src/UnauthenticatedSolidLdoProvider.tsx
    • packages/test-solid-server/src/authFetch.ts

  • Dependencies: Remove cross-fetch from 5 package.json files:

    • connected-solid, solid-react, connected-nextgraph, ldo, react

  • TypeScript config: Add "lib": ["ES2021", "DOM"] to packages/connected-solid/tsconfig.esm.json to provide DOM types (HeadersInit, window, etc.) that were previously included transitively via cross-fetch's type reference

  • Tests: Update guaranteeFetch.test.ts to assert against global fetch instead of crossFetch

Example

// Before
import crossFetch from "cross-fetch";
export function guaranteeFetch(fetchInput?: typeof fetch): typeof fetch {
  return fetchInput || crossFetch;
}

// After
export function guaranteeFetch(fetchInput?: typeof fetch): typeof fetch {
  return fetchInput || fetch;
}

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • www.w3.org
    • Triggering command: /usr/local/bin/node /usr/local/bin/node /home/REDACTED/work/ldo/ldo/node_modules/jest-worker/build/workers/processChild.js mcli/run-script/--coverage node er/b�� config.esm.json tsconfig.cjs.json (dns block)
    • Triggering command: /usr/local/bin/node /usr/local/bin/node /home/REDACTED/work/ldo/ldo/node_modules/jest-worker/build/workers/processChild.js odules/.bin/sh node er/b�� config.esm.json tsconfig.cjs.json (dns block)
    • Triggering command: /usr/local/bin/node /usr/local/bin/node /home/REDACTED/work/ldo/ldo/node_modules/jest-worker/build/workers/processChild.js es/ldo/node_modu--coverage sh k/ld�� tsc --project tsconfig.esm.json n es/.bin/sh config.cjs.json watchman node mcli/run-script/lib/node-gyp-binjest --coverage sh -c config.cjs.json && node ../../scripts/rename-to-cjs.js sh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>Remove cross-fetch usage</issue_title>
<issue_description>fetch is a global object as of Node 18; which is now the oldest supported version of Node. Thus it is safe to remove the cross-fetch global and just use fetch in the next major release.</issue_description>

Comments on the Issue (you are @copilot in this section)

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@jeswr jeswr linked an issue Jan 3, 2026 that may be closed by this pull request
Failed to load remote context when trying to run ldo build on many shex shapes #43
Open
@jeswr jeswr mentioned this pull request Jan 3, 2026
Open
@jeswr
Remove cross-fetch usage and replace with native fetch
1d956c7 
- Remove cross-fetch imports from source files and replace with global fetch
- Remove cross-fetch from package.json dependencies across 5 packages
- Update tests to use global fetch instead of cross-fetch
- Add DOM lib to connected-solid tsconfig.esm.json to provide type definitions
  for DOM types (HeadersInit, window, etc.) that were previously provided
  transitively via cross-fetch's type reference

Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove cross-fetch usage in major release Remove cross-fetch usage Jan 3, 2026
Copilot AI requested a review from jeswr January 3, 2026 13:32
@jeswr jeswr marked this pull request as ready for review January 3, 2026 13:34
Copilot AI review requested due to automatic review settings January 3, 2026 13:34
Copilot AI reviewed Jan 3, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the cross-fetch polyfill dependency from the codebase in favor of native fetch, which is now globally available as of Node 18 (the minimum supported version). The changes eliminate an unnecessary dependency and modernize the codebase.

  • Replaced all cross-fetch imports with native global fetch
  • Removed cross-fetch from 5 package.json files
  • Updated TypeScript configuration to include DOM types previously provided transitively

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/connected-solid/src/util/guaranteeFetch.ts Removed cross-fetch import and updated to use global fetch; also fixed typo "Guantees" → "Guarantees"
packages/solid-react/src/UnauthenticatedSolidLdoProvider.tsx Removed cross-fetch import and changed from libraryFetch to global fetch
packages/test-solid-server/src/authFetch.ts Removed unused cross-fetch import (already using global fetch)
packages/connected-solid/test/guaranteeFetch.test.ts Updated test assertion from crossFetch to global fetch and updated test description
packages/solid-type-index/test/fileData.ts Removed unused cross-fetch type import
packages/solid-react/test/fileData.ts Removed unused cross-fetch import
packages/connected-solid/tsconfig.esm.json Added "lib": ["ES2021", "DOM"] to provide DOM types like HeadersInit
packages/connected-solid/package.json Removed cross-fetch dependency
packages/solid-react/package.json Removed cross-fetch dependency
packages/react/package.json Removed cross-fetch dependency
packages/ldo/package.json Removed cross-fetch dev dependency
packages/connected-nextgraph/package.json Removed cross-fetch dependency
package-lock.json Updated lockfile to reflect removed dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jeswr
Copy link
Collaborator

jeswr commented Jan 3, 2026

Note that https://github.com/o-development/ldo/pull/122/files also adds a minimum version number to the package.json

@jeswr jeswr requested a review from jaxoncreed January 3, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@jeswr jeswr Awaiting requested review from jeswr

@jaxoncreed jaxoncreed Awaiting requested review from jaxoncreed

Assignees

@jeswr jeswr

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove cross-fetch usage Failed to load remote context when trying to run ldo build on many shex shapes

2 participants

@jeswr