Skip to content

refactor: resource fetcher#1023

Merged
chmjkb merged 13 commits into
mainfrom
@chmjkb/resource-fetcher-refactor
Apr 21, 2026
Merged

refactor: resource fetcher#1023
chmjkb merged 13 commits into
mainfrom
@chmjkb/resource-fetcher-refactor

Conversation

@chmjkb

@chmjkb chmjkb commented Mar 27, 2026

Copy link
Copy Markdown
Collaborator

Description

  • Extracted shared base class (BaseResourceFetcherClass) into the core package containing the fetch loop, source dispatching, and pause/resume/cancel plumbing — eliminating duplication between expo and bare
    implementations
  • Extracted platform-specific handlers (handlers.ts) for both packages, separating download mechanics from the class structure
  • Rewrote both fetchers as classes extending the base, implementing only platform-specific file system and download operations
  • Replaced the linked-list download pattern with a simple sequential for loop
  • Cancel now throws DownloadInterrupted instead of returning null, giving callers a consistent error-based API

Note: this PR still uses the legacy FS as the support for background / resumable downloads is WIP.

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

  1. Create a demo app trying to run all the functionalities exposed in Resource Fetchers
  2. Fetch function with single and/or multiple resources
  3. Cancel long fetching file, also verify fetch throws an error when its canceled
  4. Check if a model with downloaded file works
  5. Make sure deleting resources work
  6. Try running a background download

Screenshots

Related issues

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional notes

@chmjkb
chmjkb marked this pull request as ready for review March 31, 2026 10:07
@chmjkb
chmjkb force-pushed the @chmjkb/resource-fetcher-refactor branch from e6a0fc4 to 32a80a2 Compare March 31, 2026 10:36
@msluszniak

Copy link
Copy Markdown
Member

Please run yarn lint --fix in the root of the package and push changes

Comment thread packages/bare-resource-fetcher/src/handlers.ts Outdated
Comment thread packages/bare-resource-fetcher/src/handlers.ts
Comment thread packages/bare-resource-fetcher/src/handlers.ts Outdated
Comment thread packages/bare-resource-fetcher/src/ResourceFetcher.ts Outdated
Comment thread packages/bare-resource-fetcher/src/ResourceFetcher.ts
Comment thread packages/bare-resource-fetcher/src/ResourceFetcher.ts Outdated
Comment thread packages/bare-resource-fetcher/src/ResourceFetcher.ts Outdated
@chmjkb
chmjkb requested a review from mkopcins April 7, 2026 13:01
Comment thread packages/bare-resource-fetcher/src/handlers.ts

@msluszniak msluszniak 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.

I performed detailed testing, code available on branch: https://github.com/msluszniak/react-native-executorch/tree/test/pr-1023-resource-fetcher

Everything worked correctly.

@chmjkb
chmjkb merged commit da22171 into main Apr 21, 2026
3 of 4 checks passed
@chmjkb
chmjkb deleted the @chmjkb/resource-fetcher-refactor branch April 21, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resource fetcher should throw on error instead of returning empty array Remove linked list from resourcefetcher implementation

3 participants