Remove cross-fetch dependency in favor of global fetch - #134
Conversation
cross-fetch dependency in favor of global fetch
|
Supercedes #124 |
There was a problem hiding this comment.
Pull request overview
This PR removes the cross-fetch polyfill across the monorepo and switches all internal usage to the global fetch now that Node 18+ is assumed.
Changes:
- Replace
cross-fetchimports/usages with the globalfetchin runtime code and test helpers. - Update
guaranteeFetchand its test to default to globalfetch. - Remove
cross-fetchfrom packagedependencies/devDependenciesand update the workspace lockfile.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/test-solid-server/src/authFetch.ts | Drops cross-fetch import; relies on global fetch for auth flow requests. |
| packages/solid-type-index/test/fileData.ts | Removes cross-fetch type import; uses global fetch typing for helpers. |
| packages/solid-react/test/fileData.ts | Removes cross-fetch import; uses global fetch in test setup. |
| packages/solid-react/src/UnauthenticatedSolidLdoProvider.tsx | Replaces cross-fetch with global fetch in auth context provider. |
| packages/solid-react/package.json | Removes direct cross-fetch dependency. |
| packages/react/package.json | Removes direct cross-fetch dependency. |
| packages/ldo/package.json | Removes direct cross-fetch devDependency. |
| packages/connected-solid/test/guaranteeFetch.test.ts | Updates expectation to default to global fetch. |
| packages/connected-solid/src/util/guaranteeFetch.ts | Updates implementation/docs to default to global fetch. |
| packages/connected-solid/package.json | Removes direct cross-fetch dependency. |
| packages/connected-nextgraph/package.json | Removes direct cross-fetch dependency. |
| package-lock.json | Reflects dependency graph changes after removing direct cross-fetch deps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Since fetch is a global object as of Node 18 (the oldest supported version), cross-fetch is no longer needed. This removes: - cross-fetch imports from source and test files - cross-fetch from package.json dependencies in all affected packages - Updates guaranteeFetch to fall back to global fetch - Updates tests to verify against global fetch Co-authored-by: jeswr <63333554+jeswr@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
and add `DOM` to tsconfig `lib` to make up for missing polyfills previously brought by cross-fetch. Note: Adding DOM to a global config is suboptimal. It can be misleading when running LDO in Node.js and other non-browser environments. This can be improved, but requires deep restructuring of tsconfig.json across the monorepo. Possible follow-up.
f43ffc3 to
980bfca
Compare
|
I rebased this with main and resolved conflicts. Also replaced outstanding cross-fetch introduced later. Let's try to get this PR resolved. I also added Note: Adding |
|
LGTM - thanks for doing the work @mrkvon cc @jaxoncreed |
Supercedes #124. Note this is a breaking change since Node 18 will be the minimum node version supported after this change. This is not an issue since the current maintenance version of node is Node 20.
fetchis globally available since Node 18, which is now the minimum supported version. Remove thecross-fetchpolyfill across the monorepo.cross-fetchimports fromguaranteeFetch.ts,authFetch.ts,UnauthenticatedSolidLdoProvider.tsx, and test helpers insolid-type-indexandsolid-react— all now use the globalfetchcross-fetchfromdependencies/devDependenciesinconnected-solid,ldo,react,connected-nextgraph, andsolid-reactOriginal prompt
cross-fetchusage #99💡 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.