Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix JSX key attribute clobbering detected value #1042

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

comp615
Copy link

@comp615 comp615 commented Aug 5, 2024

Why am I submitting this PR

React components have a protected attribute "key" which can seemingly clobber the detected/parsed i18n key due to a naming conflict.

Does it fix an existing ticket?

Yes #1038

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • tests are included and pass: yarn test (see details here)
  • documentation is changed or added

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.35%. Comparing base (168c8d5) to head (a95a76b).
Report is 57 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
- Coverage   95.40%   95.35%   -0.05%     
==========================================
  Files          11       10       -1     
  Lines        1937     1918      -19     
==========================================
- Hits         1848     1829      -19     
  Misses         89       89              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@karellm
Copy link
Member

karellm commented Aug 10, 2024

@comp615 I have mixed feeling about this as some people might be relying on the key attribute. I'd prefer a fix that ignores the key attribute only if the i18nKey one is present. Can you update the PR?

@karellm karellm marked this pull request as draft August 10, 2024 15:38
@karellm karellm marked this pull request as ready for review August 10, 2024 15:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants