Skip to content

feat(DRC-3149): migrate Recce OSS from Axios to native Fetch#1264

Merged
kentwelcome merged 17 commits intomainfrom
feature/drc-3149-migrate-recce-oss-from-axios-to-native-fetch
Apr 2, 2026
Merged

feat(DRC-3149): migrate Recce OSS from Axios to native Fetch#1264
kentwelcome merged 17 commits intomainfrom
feature/drc-3149-migrate-recce-oss-from-axios-to-native-fetch

Conversation

@gcko
Copy link
Copy Markdown
Contributor

@gcko gcko commented Mar 31, 2026

Summary

  • Replace all Axios usage in @datarecce/ui with a thin native fetch wrapper (fetchClient.ts), eliminating Axios as a supply-chain dependency after the npm compromise
  • Migrate 32 source files and 3 test files from Axios to the new ApiClient interface
  • Fix pre-existing localStorage.clear test failure in Node 22+ environments

What Changed

New: fetchClient.ts module (js/packages/ui/src/lib/fetchClient.ts)

A thin wrapper around native fetch that provides the same call patterns as the Axios subset we used:

Component Purpose
ApiClient interface get/post/patch/delete methods matching Axios signatures
ApiResponse<T> Response shape with data, status, headers
HttpError<T> Structured error with status, data, response getter (backward compat)
isHttpError() Type guard replacing isAxiosError() with both instanceof and brand property for cross-module safety
createFetchClient() Factory with baseURL, headers, timeout, middleware (replaces interceptors)

Features: auto JSON parse/serialize, FormData detection, query param serialization, AbortSignal.timeout, request middleware, network error wrapping (status: 0).

Core migration (3 files)

  • ApiContext.tsx: axios.create() + interceptors.request.use() replaced with createFetchClient() + middleware. URL rewriting uses string ops (not new URL()) to handle both absolute and relative URLs.
  • useApiConfig.ts: Default fallback client uses createFetchClient({ baseURL: PUBLIC_API_URL })
  • RecceProvider.tsx: Props type changed from AxiosInstance to ApiClient

API module migration (22 files)

15 type-only files: AxiosInstance to ApiClient, AxiosResponse to ApiResponse
7 instance-creation files: axios.create() to createFetchClient()

Error handling migration (3 files)

  • state.ts: isAxiosError to isHttpError, error.response?.status to error.status
  • Filename.tsx: same pattern + error.response?.data?.detail to error.data?.detail
  • LineageViewOss.tsx: AxiosError instanceof to HttpError instanceof

Test migration (3 files)

  • ApiContext.test.tsx: Removed vi.mock("axios"), stub global fetch, added 7 middleware behavior tests
  • Filename.test.tsx: AxiosError fixtures to HttpError fixtures
  • LineageGraphAdapter.test.tsx: 22-line axios mock to single fetch stub

Cleanup

  • Removed axios from package.json dependencies and pnpm.overrides
  • Renamed AxiosQueryParams to QueryParams (deprecated alias kept)
  • Renamed axiosClient.ts to queryClient.ts

Bonus fix: localStorage polyfill for Node 22+

Node 22+ ships a built-in localStorage that lacks standard Storage API methods. Added a polyfill in vitest.setup.mts, fixing 48 pre-existing test failures in Filename.test.tsx.

Code Review Findings Addressed

Finding Severity Fix
new URL(path, "") crashes when baseURL is empty Critical Use path directly when baseURL is falsy
isHttpError instanceof fragile across modules Important Added __isHttpError brand property
Network TypeError not wrapped in HttpError Important Wrap fetch errors in HttpError(0, null, message)
Malformed JSON results in SyntaxError Important Wrap parseResponseBody in try/catch
FormData does not remove default Content-Type Important Explicit headers.delete("Content-Type")
Middleware new URL() crashes on relative paths Bug Switched to string manipulation

Test plan

  • pnpm type:check - 0 errors
  • pnpm lint:fix - clean
  • pnpm test - 3592 passed, 0 failed (146 files)
  • pnpm run build - successful
  • Zero axios imports in source code
  • axios removed from package.json and lockfile
  • fetchClient unit tests: 33 tests covering HTTP methods, params, headers, FormData, errors, parsing, middleware, timeout, empty baseURL, malformed JSON, network errors
  • ApiContext integration tests: 29 tests covering provider behavior, middleware URL rewriting, auth injection, memoization

Incidental formatting (non-functional)

Pre-commit hooks auto-formatted two files outside the migration scope: recce/adapter/dbt_adapter/__init__.py (single→double quotes in getattr() calls) and tests/adapter/dbt_adapter/test_dbt_cll.py (line wrapping for long assertions). No logic changes — just style normalization triggered by the commit hooks.

Closes DRC-3149

🤖 Generated with Claude Code

claude and others added 14 commits March 31, 2026 16:31
…ttpError

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…ApiResponse

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…o createFetchClient

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…o QueryParams

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Remove axios mocking entirely, stub global fetch instead. Replace
interceptors assertion with HTTP method checks. Add middleware
integration tests for apiPrefix URL rewriting and auth token injection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Replace all AxiosError fixture creation with HttpError constructor.
Add vitest alias for @datarecce/ui/lib/fetchClient.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…pter

Replace the entire vi.mock("axios") block with a single
vi.stubGlobal("fetch") call to prevent real network requests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…eryClient

Remove axios from all package.json files (dependencies and pnpm.overrides),
rename axiosClient.ts to queryClient.ts, and update all imports throughout
the codebase. This completes the axios-to-fetch migration.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
- Fix empty baseURL crash (CRITICAL): handle baseURL='' by using path directly
- Add brand property to HttpError for cross-module instanceof safety
- Wrap response body parsing to convert SyntaxError to HttpError
- Delete Content-Type header for FormData bodies
- Add tests for empty baseURL, malformed JSON, brand check

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
… with Axios

Network-level failures (TypeError for DNS/connection, AbortError for timeouts)
are now wrapped in HttpError with status=0, so isHttpError() returns true for
ALL request failures — matching Axios behavior where isAxiosError() returned
true for network errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
…ssing tests

- Fix middleware crash when baseURL is empty and apiPrefix/authToken is set:
  new URL() requires absolute URL, switched to string manipulation matching
  the original Axios interceptor approach
- Add ApiContext middleware tests: exact /api rewrite, non-/api passthrough,
  combined apiPrefix+authToken, empty baseUrl with authToken, no-middleware
- Add fetchClient tests: POST with no body, FormData removing default
  Content-Type, empty baseURL with params

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
Node 22+ provides a built-in localStorage that is a plain object without
standard Storage API methods (getItem, setItem, clear, etc.). This caused
Filename.test.tsx to fail with localStorage.clear is not a function.

Add a Storage polyfill in vitest.setup.mts that detects the incomplete
built-in and replaces it with a Map-backed implementation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
@gcko gcko self-assigned this Mar 31, 2026
@gcko gcko added the High priority Created by Linear-GitHub Sync label Mar 31, 2026
@gcko gcko requested a review from kentwelcome March 31, 2026 10:57
Copy link
Copy Markdown
Member

@kentwelcome kentwelcome left a comment

Choose a reason for hiding this comment

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

LGTM

@kentwelcome kentwelcome merged commit be1bd16 into main Apr 2, 2026
7 checks passed
@kentwelcome kentwelcome deleted the feature/drc-3149-migrate-recce-oss-from-axios-to-native-fetch branch April 2, 2026 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants