build(deps): Bump android Appcompat to 1.4.1 - #33072

Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom
gabrieldonadel:feat/bump-appcompat
Closed

build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom
gabrieldonadel:feat/bump-appcompat

Conversation

@gabrieldonadel

@gabrieldonadelgabrieldonadel commented Feb 9, 2022•
edited by cortinico
Loading

Copy link
Copy Markdown
Collaborator

Summary

Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.

Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)

Closes#31620

Changelog

[Android] [Changed] - Bump android Appcompat to 1.4.1

Test Plan

Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app

@facebook-github-botfacebook-github-bot added 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. labels Feb 9, 2022
gabrieldonadel
gabrieldonadel commented Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image

@react-native-botreact-native-bot added the Platform: Android Android applications. label Feb 9, 2022
@gabrieldonadelgabrieldonadel mentioned this pull request Feb 9, 2022
Closed
@analysis-bot

analysis-bot commented Feb 9, 2022•
edited
Loading

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
ios-universaln/a--

Base commit: 172f990
Branch: main

@analysis-bot

analysis-bot commented Feb 9, 2022•
edited
Loading

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237

Base commit: 172f990
Branch: main

@gabrieldonadel

gabrieldonadel commented Feb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753

@gabrieldonadel

gabrieldonadel commented Feb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Ohh never mind, I guess you already have a PR fixing this

@facebook-github-bot

facebook-github-bot commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

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

@cortinico

cortinico commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

Ohh never mind, I guess you already have a PR fixing this

Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍

@ShikaSD

ShikaSD commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?

https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538

@gabrieldonadel

gabrieldonadel commented Feb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?

https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538

Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?

@ShikaSD

ShikaSD commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

Should I run some command that updates this automatically or do I need to get it from somewhere else?

It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.

@gabrieldonadel

gabrieldonadel commented Feb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.

Ohh got it! Thanks @ShikaSD

@gabrieldonadel

gabrieldonadel commented Feb 9, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?

Done 😄

@facebook-github-bot

facebook-github-bot commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

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

@ShikaSD

ShikaSD commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

I think CI is broken after the latest commit, it cannot find some androidx classes:

stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico

cortinico commented Feb 9, 2022

Copy link
Copy Markdown
Contributor

#33068 has been marged. You should be able to rebase and get a green CI from that 👍

@gabrieldonadel

gabrieldonadel commented Feb 10, 2022

Copy link
Copy Markdown
CollaboratorAuthor

I think CI is broken after the latest commit, it cannot find some androidx classes:

Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now

@gabrieldonadel

gabrieldonadel commented Feb 10, 2022

Copy link
Copy Markdown
CollaboratorAuthor

I think CI is broken after the latest commit, it cannot find some androidx classes:

stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?

I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out

@cortinico

cortinico commented Feb 10, 2022

Copy link
Copy Markdown
Contributor

Any suggestion on how to fix this @ShikaSD@cortinico ?

Yup, you'll need to add to the BUCK file you attached the following rule:

fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:

fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)

If you see the other targets have similar definition

@gabrieldonadel

gabrieldonadel commented Feb 10, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Yup, you'll need to add to the BUCK file you attached the following rule

Adding that actually ends up breaking another place now
image

@gabrieldonadel

gabrieldonadel commented Feb 10, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Yup, you'll need to add to the BUCK file you attached the following rule

Adding that actually ends up breaking another place now

I managed to fix this by bumping android:core version as well

@facebook-github-bot

facebook-github-bot commented Feb 10, 2022

Copy link
Copy Markdown
Contributor

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

@gabrieldonadel
gabrieldonadel deleted the feat/bump-appcompat branch February 10, 2022 18:06
@react-native-bot

react-native-bot commented Feb 10, 2022

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @gabrieldonadel in 6b61995.

When will my fix make it into a release? | Upcoming Releases

@react-native-botreact-native-bot added the Merged This PR has been merged. label Feb 10, 2022
rasaha91 pushed a commit to rasaha91/react-native-macos that referenced this pull request Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550
This was referenced Jul 7, 2022
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees

No one assigned

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
build(deps): Bump android Appcompat to 1.4.1 - #33072
Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Closed
build(deps): Bump android Appcompat to 1.4.1#33072
gabrieldonadel wants to merge 5 commits into
react:mainfrom 
gabrieldonadel:feat/bump-appcompat
Conversation
@gabrieldonadel
@gabrieldonadelgabrieldonadel
commented
Feb 9, 2022•

edited by cortinico

Loading

Copy link


Copy Markdown

Collaborator
Summary
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to #33065)
Closes#31620
Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Test Plan
Use ./scripts/test-manual-e2e.sh to test both RNTester and a new app
@facebook-github-botfacebook-github-bot
added

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.
labels
Feb 9, 2022
gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022
View reviewed changes
Comment threadReactAndroid/src/main/java/com/facebook/react/devsupport/RedBoxDialog.java
Outdated

@gabrieldonadelgabrieldonadelFeb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

RNTester was crashing without this extra check
image
@react-native-botreact-native-bot
added
the 
Platform: Android
Android applications.
label
Feb 9, 2022
@gabrieldonadelgabrieldonadel
mentioned this pull request
Feb 9, 2022
 Closed

@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
ios-universaln/a--
Base commit: 172f990

Branch: main
@analysis-bot
analysis-bot
commented
Feb 9, 2022•

edited

Loading

Copy link


Copy Markdown

PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,188,751+68,175
androidhermesarmeabi-v7a7,790,036+69,059
androidhermesx868,559,456+68,782
androidhermesx86_648,511,320+68,318
androidjscarm64-v8a9,857,741+70,108
androidjscarmeabi-v7a8,843,894+70,978
androidjscx869,825,019+70,689
androidjscx86_6410,420,988+70,237
Base commit: 172f990

Branch: main
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
@cortinico I believe this commit cd79317 broke CI, in particular the test_android_template workflow https://app.circleci.com/pipelines/github/facebook/react-native/12051/workflows/2592c9f6-217f-4595-9981-82f4097fecf3/jobs/234753
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Ohh never mind, I guess you already have a PR fixing this
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Ohh never mind, I guess you already have a PR fixing this
Yup you're right. We're working on it, hopefully it will be out in the next hours. Sorry for the disruption 👍
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
https://github.com/facebook/react-native/blob/369a7ab5d332be945137aaa487f0c09fc7fa8c57/ReactAndroid/src/main/third-party/android/androidx/BUCK#L538
Sure @ShikaSD, I can update that. My only question is regarding the sha1 value, should I run some command that updates this automatically or do I need to get it from somewhere else?
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
Should I run some command that updates this automatically or do I need to get it from somewhere else?
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
It can be retrieved from google maven repo, I don't think we have any command that upgrades it automatically. If you struggle to find the hash, you can upgrade the version and then wait for the CI to fail, it should have a new hash there.
Ohh got it! Thanks @ShikaSD
@gabrieldonadel
gabrieldonadel
commented
Feb 9, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Hi, thanks for upgrading it, do you mind updating the BUCK file for the dependency as well?
Done 😄
@facebook-github-bot
facebook-github-bot
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@ShikaSD
ShikaSD
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

@cortinico
cortinico
commented
Feb 9, 2022

Copy link


Copy Markdown

Contributor
#33068 has been marged. You should be able to rebase and get a green CI from that 👍
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
Yeah, I had quite a hard time installing BUCK in my machine to investigate this because I'm using an M1 mac, I finally got it working, I'll start investigating why this is falling now
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
I think CI is broken after the latest commit, it cannot find some androidx classes:
stderr: : warning: unknown enum constant androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX
/root/react-native/ReactAndroid/src/main/java/com/facebook/react/views/unimplementedview/ReactUnimplementedView.java:22: error: cannot access androidx.core.widget.TintableCompoundDrawablesView
mTextView.setLayoutParams(

Any suggestion on how to fix this @ShikaSD@cortinico ?
I believe the cannot access androidx.core.widget.TintableCompoundDrawablesView error is build caused by some sort of version mismatch in the BUCK file, but I can't quite figure it out
@cortinico
cortinico
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
Any suggestion on how to fix this @ShikaSD@cortinico ?
Yup, you'll need to add to the BUCK file you attached the following rule:
fb_native.android_prebuilt_aar(
name = "appcompat-resources-binary",
aar = ":appcompat-resources-binary-aar",
)

and edit the "appcompat" target with:
fb_native.android_library(
name = "appcompat",
visibility = ["PUBLIC"],
exported_deps = [
":annotation",
":appcompat-binary",
+ ":appcompat-resource-binary",
":collection",
":core",
":cursoradapter",
":fragment",
":legacy-support-core-utils",
":vectordrawable",
":vectordrawable-animated",
],
)
If you see the other targets have similar definition
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
image
@gabrieldonadel
gabrieldonadel
commented
Feb 10, 2022

Copy link


Copy Markdown

CollaboratorAuthor
Yup, you'll need to add to the BUCK file you attached the following rule
Adding that actually ends up breaking another place now
I managed to fix this by bumping android:core version as well
@facebook-github-bot
facebook-github-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Contributor
@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@gabrieldonadel
gabrieldonadel
deleted the

feat/bump-appcompat

branch
February 10, 2022 18:06
@react-native-bot
react-native-bot
commented
Feb 10, 2022

Copy link


Copy Markdown

Collaborator
This pull request was successfully merged by @gabrieldonadel in 6b61995.
When will my fix make it into a release? | Upcoming Releases
@react-native-botreact-native-bot
added
the 
Merged
This PR has been merged.
label
Feb 10, 2022
rasaha91 pushed a commit
to rasaha91/react-native-macos
that referenced
this pull request
Jul 7, 2022
@gabrieldonadel@rasaha91
build(deps): Bump android Appcompat to 1.4.1 (react#33072)
624a5ab
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to react#33065)
Closesreact#31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: react#33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550

This was referenced Jul 7, 2022
 Merged

 Merged

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

Reviewers

@cortinicocortinicoAwaiting requested review from cortinico

Assignees


No one assigned


Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@cortinico@ShikaSD@react-native-bot