Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix fontWeight again#806

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
cknitt merged 1 commit intomainfromfix-font-weight-again
Jul 1, 2024
Merged

Fix fontWeight again#806

cknitt merged 1 commit intomainfromfix-font-weight-again
Jul 1, 2024

Conversation

@cknitt
Copy link
Member

Fixes#775 again by reverting thefontWeight type definition to its pre-#793 state.

Unfortunately this is a breaking change yet again. But better than crashes on Android...

fhammerschmidt
fhammerschmidt previously approved these changesMar 4, 2024
@cknittcknittforce-pushed thefix-font-weight-again branch froma134bc7 to239d39aCompareMarch 4, 2024 12:26
@Freddy03h
Copy link
Member

Freddy03h commentedMar 7, 2024
edited
Loading

@cknitt which version ofreact-native are you using?

Number values are accepted since0.71 :facebook/react-native#34598

I didn't experience any crash on Android or iOS.

@cknitt
Copy link
MemberAuthor

That's weird as I had the crash on Android with 0.73.5.

@Freddy03h
Copy link
Member

I'm still on 0.71.14 so maybe there is a regression on this inreact-native@0.73 but it's seems that those modifications and the tests still exists onmain branch… so I don't know ^^'

@Freddy03h
Copy link
Member

I tested it on a 0.73.4 project and it work well on Android, without crashing.

@cknitt
Copy link
MemberAuthor

Sorry, patched this in the project I was working on at the time and then lost track of it.

Now a colleague has encountered the same problem in another project with RN 0.74.2. So this is definitely still an issue. Will merge now.

@cknittcknitt merged commit3629eb9 intomainJul 1, 2024
@cknittcknitt deleted the fix-font-weight-again branchJuly 1, 2024 11:42
@Freddy03h
Copy link
Member

I still not experienced the issue since 0.71

Are you using a custom font?

Are you using the new architecture?

@Freddy03h
Copy link
Member

@cknitt Hi!

  • I tried using number on a 0.74 project and it doesn't crash on android.
  • I also never experienced a crash linked to this issue on my project with 25k users/day and a variety of android versions and configuration.
  • Tests with numbers existing on the react-native project.
  • I don't find any related issues on the RN project.

It's seems to be definitively an issue… but only on your project? Can you repro the issue outside?

This type was change 1 year ago, no one else seems to experienced the issue since, and I'm worried the revert to the old type will annoy people…

@cknitt
Copy link
MemberAuthor

This is not specific to a particular project of ours. My colleague encountered the issue in a new project, not in the one where I originally experienced it.

I will try to create a repo with a simple reproduction.

@cknitt
Copy link
MemberAuthor

cknitt commentedJul 3, 2024
edited
Loading

@Freddy03h Ok, here is a simple repro:

  1. Create a new RN project withnpx react-native init.
  2. Run the Android project.
  3. At the bottom of App.tsx, in thehighlight style, change fontWeight from'700' to700.

The problem does not occur for the other styles in that file.
Maybe it is because the highlight style is applied on a nested<Text>.

Screenshot_1720004134

@Freddy03h
Copy link
Member

@cknitt Hi! Thank you for all the informations! I opened an issue an thereact-native repofacebook/react-native#45285 and the error only happen in Dev mode with the use ofStylesheet.create and without the use of an array in thestyle prop.

I never experienced the issue on my project because now with dynamic style (dark/light mode) there's always a variable color to dynamically pass to the style prop in an array.

And because the error is not effective on Release mode, I never saw it on crashlytics (even with 2 text in my entire app where the issue should happen).

It's possible to create a patch-package with this tiny fix that will be fixed soon I hope :https://github.com/facebook/react-native/pull/45299/files

So, what's your opinion about reverting the type to the polymorphic variant? I think it would be nice not to have to rewrite the type for everyone for an issue that happen only in Dev mode in the old architecture.

Thank you.

@cknitt
Copy link
MemberAuthor

cknitt commentedJul 6, 2024
edited
Loading

Hi@Freddy03h! Thank you very much for opening the issue on the React Native repo to get to the bottom of this.

It's possible to create a patch-package with this tiny fix that will be fixed soon I hope :https://github.com/facebook/react-native/pull/45299/files

I can confirm that this fixes the issue. This solution is fine with me, I'll make a PR to revert the type to the polymorphic variant.

Freddy03h reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fhammerschmidtfhammerschmidtfhammerschmidt approved these changes

@Freddy03hFreddy03hAwaiting requested review from Freddy03h

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fontWeight broken in master

4 participants

@cknitt@Freddy03h@fhammerschmidt

[8]ページ先頭

©2009-2025 Movatter.jp