Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ToolbarAndroid removed in 0.61 #26591

Closed
thomasvm opened this issue Sep 26, 2019 · 33 comments
Closed

ToolbarAndroid removed in 0.61 #26591

thomasvm opened this issue Sep 26, 2019 · 33 comments
Labels
Bug Component: ToolbarAndroid Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@thomasvm
Copy link

React Native 0.61 removed ToolbarAndroid from the provided components. This is not mentioned in the release docs and the component was not extracted into community package either. So there is no clear "path forward". Also, ToolbarAndroid is still mentioned as a provided components in the latest docs.

https://facebook.github.io/react-native/docs/toolbarandroid

React Native version: 0.61
System:
OS: Windows 10
CPU: (4) x64 Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
Memory: 3.51 GB / 15.88 GB
Binaries:
Node: 10.11.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.12.1 - ~\AppData\Roaming\npm\yarn.CMD
npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
IDEs:
Android Studio: Version 3.5.0.0 AI-191.8026.42.35.5791312

Steps To Reproduce

  1. Upgrade project to react-native 0.61
  2. Code that is using ToolbarAndroid fails with error message "Invariant Violation: Element type is invalid: expected a string (for built-in components) or a class/funcction"
@Smoothsmith
Copy link

Smoothsmith commented Oct 8, 2019

I found the commit that removes it (Or at least, part of it):
31ab947#diff-39808e06333026f9ddb02287226012e1

But I don't know how to use that information to access/continue using it (Which our app does need to do - Hoping for a solution that doesn't involve rewriting parts of the app).

@thomasvm
Copy link
Author

thomasvm commented Oct 8, 2019

@Smoothsmith There is another commit that removes the Java part to FB internal
cbbbb45

What I ended up doing - and what I guess will be your best shot at the moment - is to include the JavaScript and Java code in our own codebase (with adjusted namespaces etc) and consume it from there.

@forki
Copy link
Contributor

forki commented Dec 20, 2019

@hramos @cpojer Is there an official replacement? It's still in the docs and hit uns really hard.

@connectdotz
Copy link

hmmm... this seems like a major API breakage without any mention in changelog or blog, makes one wonder if this is an intentional change 🤔 ? We are already at 0.61.5 and there is still no official guidance on this issue... does the react-native core team not care about the tooling community or adoption of the new release? It would be great to have at least a discussion...

@Naturalclar
Copy link
Contributor

@connectdotz I may be wrong, but I think removal of ToolbarAndroid has been discussed as part of a Lean Core, and I believe it was considered deprecated since RN 0.59

@forki
Copy link
Contributor

forki commented Jan 4, 2020

@Naturalclar deprecation is not in the release notes.
and the toolbar is still in the docs. see https://facebook.github.io/react-native/docs/toolbarandroid

@connectdotz
Copy link

I stand corrected, @Naturalclar thanks for the link, it provided the missing context for me, but it only further deepened the puzzle of how ToolbarAndroid was removed...

After reading PR #24999, which enabled the removal of ToolbarAndroid, it became more clear how ToolbarAndroid could be replaced. But the migration is not clearly documented, to say the least... I understand this is just a "convenient" class and there is nothing special but a composite of core building blocks... However, the same thing can be said for many of the to-be-deprecated classes in the lean core initiative... again, I am not sure why ToolbarAndroid is being handled differently, i.e. it was removed without any clear documentation on migration path nor grace period for 3rd party libraries and user applications to adapt?

Will @cpojer and @ericlewis consider updating the document to clarify the migration approach? Better yet, maybe even bring back the ToolbarAndroid and deprecate it later with the rest of the lean core classes?

@ericlewis
Copy link
Contributor

ericlewis commented Jan 5, 2020

One could reimplement it as it’s own module based on what was removed.

This was quite a while ago. But if I recall the reasoning correctly, it was that it’s been long deprecated with the idea that more complete navigation solutions better implement this sort of thing. (And it’s easy to just copy paste in to your own project)

@connectdotz
Copy link

connectdotz commented Jan 5, 2020

@ericlewis I completely agree that these kind of convenient components should live outside of react-native core, just that the removal approach for ToolbarAndroid is inconsistent with the rest of lean-core initiative components, not to mention the migration barrier it has imposed: first, you will get puzzling compile error, but nothing is mentioned in changelog, release note, and the official document didn't say anything should be different... well, you got the picture...

Furthermore, If ToolbarAndriod is a common component, I suspect it is, instead of everybody duplicates the code you proposed, it would probably be better to create an external package, as many of the lean-core components moving toward, no?

@ericlewis
Copy link
Contributor

ericlewis commented Jan 5, 2020

I don’t recall it being rushed, since it was removed probably close to a year ago and the discussion about it being public.

Anyone from the community is free to implement their own version of this and provide it as open source.

They are also free to open pull requests to update documentation.

I encourage doing both things!

@forki
Copy link
Contributor

forki commented Jan 5, 2020

@ericlewis the problem is not that it is removed. The problem is that it's missing in the release notes and the docs still mentions it as supported

@elicwhite
Copy link
Member

The changelog generator probably skipped this commit because the title includes the word “internal”. This should definitely be in the changelog. Can someone send a PR to the changelog please?

@cpojer
Copy link
Contributor

cpojer commented Jan 6, 2020

At the time it was my understanding that ToolbarAndroid had been deprecated for a while and was a rarely used component that can be implemented purely in JavaScript using styles. That's why I decided to remove it altogether without asking others to help maintain it as a separate new module.

If this is indeed a module that people still heavily rely on, here are some next steps:

  • Copy the source to a new repository and make it into a third-party repo OR provide a new solution using JavaScript only that mirrors the same functionality.
  • Update the changelog which missed this removal (sorry about that, as Eli says it was likely filtered).
  • Remove the existing documentation from the website.

@forki
Copy link
Contributor

forki commented Jan 6, 2020

"purely Javascript"? Are you sure?

@cpojer
Copy link
Contributor

cpojer commented Jan 6, 2020

Here is an example implementation by @ericlewis which we are using in RNTester: #24999

@forki
Copy link
Contributor

forki commented Jan 6, 2020

But that's not the native control, right? And it's also not showing ActionIcons and how ActionIcons overflow.

@forki
Copy link
Contributor

forki commented Jan 6, 2020

@thomasvm any chance you can open source that part that you have extracted?

@connectdotz
Copy link

connectdotz commented Jan 6, 2020

thanks, @cpojer for swift actions.

I wonder, instead of removing the document, will it be more helpful to state at the top that it is deprecated since 0.61(?) and suggest a way forward for both older and current version users?

Maybe I am still missing some context... wasToolbarAndroid deprecated a long time ago? I bump into this issue with a new react-native app (v0.61.5) with react-native-vector-icons and react-native-gesture-handler, both have ToolbarAndriod dependencies. These 3rd party libraries are quite popular among react-native communities. Judging they all still have dependencies on this class (react-native-gesture-handler did merge the removal PR just 4 days ago but haven't released the npm package yet), I got the impression this is a fairly new issue(?)...

@forki observation is revealing that perhaps a pure javascript solution is not quite fully functional-equivalent yet? Maybe a little bit more time for the new solution to be fully tested and mature, in a 3rd party library before deprecating the ToolbarAndroid would be safer?

On the other hand, if this is indeed a rarely used component, then one can argue all 3rd party libraries should just remove it rather than restoring it in the core library during the transition. I don't know how to determine ToolbarAndroid popularity on behave of the 3rd party libraries and the communities, but nevertheless it is probably best to give the library/app owners some warning and time to decide how to support, or not, about this component before removing it from react-native?

@cpojer
Copy link
Contributor

cpojer commented Jan 7, 2020

@connectdotz I definitely dropped the ball on this one a little bit, and I'm sorry.

Regarding gesture-handler, I did work with @kmagiera to get the project ready. software-mansion/react-native-gesture-handler#617 was the result of that back at the same time when ToolbarAndroid was removed so that version of the gesture handler should work with both the old and new version of React Native.

We will not be restoring this within react-native so the recommendation is to make a new repository with this functionality.

@cesardeazevedo
Copy link

Anyone already started moving ToolbarAndroid to a new repository? My library react-native-collapsing-toolbar highly relied on the native implementation.

@connectdotz
Copy link

connectdotz commented Jan 7, 2020

@cpojer thanks for the explanation, no apology needed, just trying to understand the whole story so we can better plot the path forward.

With the information you mentioned above, I was able to draw a more clear picture now. It seems the compile error for projects that didn't use ToolbarAndroid started to be reported after necolas/react-native-web#512, which removed ToolbarAndroid support around 18 days ago for react-native-web@0.12.0-rc.x. Apps with the latest versions (react-native@0.61.x, react-native-web@0.12.0-rc.x) started to see compile error due to 3rd party libraries are linked against users' own react-native library in the react-native-web paradigm, which the regular react-native apps won't suffer unless they use ToolbarAndroid directly...

I understand the decision not to restore ToolbarAndroid, that's fine, then we should get the external library established ASAP, can you create the react-native-community/toolbar-android repo and maybe move the original ToolbarAndriod code over? Hmmm... actually, I wonder wouldn't it be more consistent and speedy to just take the whole to-be-deprecated components into its own monorepo package, something like lean-components, which should have a clear mandate to have only the react and react-native dependency, distinguished from all the other 3rd party libraries out there? This way you guys can speed up the lean core initiative progress without blocked by community ownership as well as providing a more consistent structure/process around these components 🤔 ...

Anyway, for people who landed here looking for surviving react-native-web and ToolbarAndroid deprecation, my current workaround is to downgrade react-native-web to ^0.11: it appeared to avoid the compile error, even with react-native@0.61.x, as long as you don't use ToolbarAndroid directly. Hopefully, we will have a comprehensive solution soon 🤞

@cpojer
Copy link
Contributor

cpojer commented Jan 8, 2020

@connectdotz I created https://github.com/react-native-community/toolbar-android and gave you access. Feel free to send PRs there, merge them and publish a new package.

@connectdotz
Copy link

@cpojer that was clever 😏 ok, I will shut up and actually do something to resolve this issue 😊

I will take a shot at seeding the repo with the original ToolbarAndriod. We can maybe look to include @ericlewis's alternative js implementation in the following release...

@ericlewis
Copy link
Contributor

ericlewis commented Jan 9, 2020

@connectdotz You know what, I was a bit confused by the confusion until @cpojer chimed in. I did indeed create a pure JS version of this which completely mimicked the abilities needed by the RNTester app (as well as tests). I think it might live there? Either way, sorry for the inconvenience!

Also, I am pretty sure this was never a proper supported component, but was along the lines of NavigatorIOS, which is something that is also still documented and definitely deprecated and probably removed now right? So @cpojer @TheSavior we might wanna fix that as its still in the docs and seems well documented!

@ericlewis
Copy link
Contributor

We somehow dropped the ball here, I would assure you that you can definitely achieve fantastic results with a pure javascript solution!

@forki
Copy link
Contributor

forki commented Jan 9, 2020

From a user's perspective there were absolutely no signs that this standard Android component is not treated as something properly supported. There was no such hint in the docs. There are even fixes documented in the release notes.
Regarding full js solution: I think the currently proposed way is a good way forward. Extraction of the deleted code into a react-native-community control will unblock people. Afterwards it may or may not be improved.

@connectdotz
Copy link

connectdotz commented Jan 10, 2020

@cpojer I am not able to add files to https://github.com/react-native-community/toolbar-android... no permission to commit directly nor forking because it is empty... maybe adding a dummy file like README.md? Thanks.

[update]
never mind, just saw your invite, sorry.

@connectdotz
Copy link

@ericlewis

I did indeed create a pure JS version of this which completely mimicked the abilities needed by the RNTester app (as well as tests). I think it might live there?

I think we can definitely host both implementations in the repo after the initial release (with the extracted code). The new pure js implementation can be named something likeToolbarAndroidJS so people can experiment with it safely until it is mature.

I am pretty sure this was never a proper supported component

Do you care to elaborate?

@connectdotz
Copy link

ok, initial check-in is done. However, I only tested it with RN 0.60 and 0.61.5 (see examples there), which went smoothly with autolink. I don't have older RN repo to test with the manual setup, if you do please try it and see if we need to update the README.md or something else...

I have check in the npm package react-native-community-toolbar-android-0.1.0.tgz in the repo but haven't published yet, thinking it surely needs a bit more testing than the examples there...

You can install the package from the tgz file, for example yarn add file:react-native-community-toolbar-android-0.1.0.tgz. Feel free to file issues in https://github.com/react-native-community/toolbar-android so we can get the package ready ASAP. Thanks.

@connectdotz
Copy link

connectdotz commented Jan 11, 2020

@cpojer circleci won't let me add @react-native-community/toolbar-android 😞 The config file should be ready to go, just need the react-native-community admin to register the project in circleci, do you or other admin mind registering it for us? thx.

@cpojer
Copy link
Contributor

cpojer commented Jan 13, 2020

Should be all set :)

facebook-github-bot pushed a commit that referenced this issue Feb 3, 2020
Summary:
Add warning message for development that is trying to use ToolbarAndroid that has been removed from ReactNative in 0.61

Ref: #26591

## Changelog

[General] [Deprecated] - Add warning message for trying to use ToolbarAndroid which has been removed from the core.
Pull Request resolved: #27930

Differential Revision: D19690581

Pulled By: cpojer

fbshipit-source-id: 8e404fe62651fba4940199c74c45270d6e853740
@khizerism
Copy link

As im new to react-native, this is the best solution i found so far, hope it helps.

https://github.com/react-native-community/toolbar-android

First, add npm install @react-native-community/toolbar-android --save or yarn lover add yarn add @react-native-community/toolbar-android

Second, Run react-native start command.

Third, Run react-native run-android

Happy coding :)

osdnk pushed a commit to osdnk/react-native that referenced this issue Mar 9, 2020
Summary:
Add warning message for development that is trying to use ToolbarAndroid that has been removed from ReactNative in 0.61

Ref: facebook#26591

## Changelog

[General] [Deprecated] - Add warning message for trying to use ToolbarAndroid which has been removed from the core.
Pull Request resolved: facebook#27930

Differential Revision: D19690581

Pulled By: cpojer

fbshipit-source-id: 8e404fe62651fba4940199c74c45270d6e853740
@thomasvm
Copy link
Author

Since ToolbarAndroid is now extracted into a separate library and now that the documentation mentions this, I am going to close this issue. Thanks for the work everybody!

@facebook facebook locked as resolved and limited conversation to collaborators Oct 3, 2021
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Component: ToolbarAndroid Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests