Skip to content
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.

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

feat(image): enabling basic cache control for android #47182

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mateoguzmana
Copy link
Contributor

Summary:

Fixes #12606

Previously, Image cache control options were not functional on Android, even though they were being passed to the native component via the source prop. This PR addresses that by implementing logic to manage cache behaviour on Android.

When the reload option is explicitly set, the image is now evicted from both memory and disk caches before a new request is made. This ensures the image is always fetched from the source, aligning the caching behaviour between Android and iOS for the default and reload options.

Changelog:

[ANDROID][ADDED] - Enabling basic Image cache control for Android

Test Plan:

Added a new example to the rn-tester, where we can notice that the image on the right is reloaded if rendered or re-rendered as the cache policy is set to reload. The image on the left has the cache policy set to default and won't be re-rendered as the image is already in the cache. See the video below:

Screen.Recording.2024-10-23.at.23.31.02.mov

Also tested on both old and new architecture.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 23, 2024
@react-native-bot
Copy link
Collaborator

Fails
🚫

Danger failed to run dangerfile.js.

Error ReferenceError

validateChangelog is not defined
ReferenceError: validateChangelog is not defined
    at Object.<anonymous> (dangerfile.js:48:18)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at requireFromString (/home/runner/work/react-native/react-native/node_modules/require-from-string/index.js:28:4)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:161:68
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:52:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:33:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:27:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:23:12)
    at runDangerfileEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:118:132)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:181:38
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:44:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:25:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:19:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:15:12)
    at Object.executeRuntimeEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:144:88)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:101:47
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:34:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:15:53)
    at fulfilled (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:6:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Dangerfile

43|     'This will require a manual import by a Facebook employee.';
44|   warn(`${title} - <i>${idea}</i>`);
45| }
46| 
47| // Provides advice if a test plan is missing.
--------------------^
48| const includesTestPlan =
49|   danger.github.pr.body &&
50|   danger.github.pr.body.toLowerCase().includes('## test plan');
51| if (!includesTestPlan && !isFromPhabricator) {

Generated by 🚫 dangerJS against c2b0e6c

@mateoguzmana mateoguzmana marked this pull request as ready for review October 23, 2024 22:10
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 23, 2024
@facebook-github-bot
Copy link
Contributor

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Oct 25, 2024
Summary:
Danger seems to have a bug where it's not transpiling the import of
rnx-kit/rn-changelog-generator. This mitigates the issue to get our
project back on track.

This can be replicated locally by:

```bash
DEBUG="*" DANGER_GITHUB_API_TOKEN=$GITHUB_TOKEN yarn danger pr #47182
```
You can see it running correctly here when switching to the branch with the fix.  **I'm a little concerned that this is still failing on the PR**.  Thoughts?

 {F1946190275}

Changelog: [internal]

Pull Request resolved: #47192

Reviewed By: cortinico

Differential Revision: D64924466

Pulled By: blakef

fbshipit-source-id: 68df0521620809effe3a78ce842e043382ad64a6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image never reloads with same uri
3 participants