Skip to content

fix: parse Image srcSet densities consistently#56890

Closed
ya-nsh wants to merge 1 commit into
facebook:mainfrom
ya-nsh:fix/image-srcset-density-parsing
Closed

fix: parse Image srcSet densities consistently#56890
ya-nsh wants to merge 1 commit into
facebook:mainfrom
ya-nsh:fix/image-srcset-density-parsing

Conversation

@ya-nsh
Copy link
Copy Markdown
Contributor

@ya-nsh ya-nsh commented May 19, 2026

Summary

  • parse Image srcSet entries even when there is no space after the comma
  • keep fractional density descriptors like 1.5x
  • keep bare x descriptors out of the source list

Changelog:

[General] [Fixed] - Parse Image srcSet density descriptors consistently

Test Plan

  • yarn jest packages/react-native/Libraries/Image/__tests__/ImageSourceUtils-test.js --runInBand
  • npx prettier --check packages/react-native/Libraries/Image/ImageSourceUtils.js packages/react-native/Libraries/Image/__tests__/ImageSourceUtils-test.js
  • yarn flow-check

@meta-cla meta-cla 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 May 19, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label May 19, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented May 19, 2026

@javache has imported this pull request. If you are a Meta employee, you can view this in D105689719.

);
} else {
const scale = parseInt(xScale.split('x')[0], 10);
const scale = Number(xScale.slice(0, -1));
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.

Number('') returns 0 (not NaN), so a bare 'x' descriptor (no preceding number) now silently produces scale: 0 instead of being filtered by the isNaN check.

Maybe use parseFloat() instead of Number()? parseFloat('') returns NaN (matching the old guard behavior) while parseFloat('1.5') still correctly returns 1.5.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks. I switched this to parseFloat and added a regression case for the bare x descriptor so it stays filtered out.

I also fixed the changelog heading in the PR body.

Validated with:

  • yarn jest packages/react-native/Libraries/Image/__tests__/ImageSourceUtils-test.js --runInBand
  • npx prettier --check packages/react-native/Libraries/Image/ImageSourceUtils.js packages/react-native/Libraries/Image/__tests__/ImageSourceUtils-test.js
  • yarn flow-check

Updated commit: 5c47a49

@ya-nsh ya-nsh force-pushed the fix/image-srcset-density-parsing branch from 07f60e3 to 5c47a49 Compare May 19, 2026 15:04
@meta-codesync meta-codesync Bot closed this in e4cf0a1 May 20, 2026
@facebook-github-tools facebook-github-tools Bot added the Merged This PR has been merged. label May 20, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented May 20, 2026

@javache merged this pull request in e4cf0a1.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @ya-nsh in e4cf0a1

When will my fix make it into a release? | How to file a pick request?

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. Merged This PR has been merged. 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.

3 participants