Export the DevSettings module, add `addMenuItem` method #25848

janicduplessis · 2019-07-27T20:24:07Z

Summary

I wanted to configure the RN dev menu without having to write native code. This is pretty useful in a greenfield app since it avoids having to write a custom native module for both platforms (and might enable the feature for expo too).

This ended up a bit more involved than planned since callbacks can only be called once. I needed to convert the DevSettings module to a NativeEventEmitter and use events when buttons are clicked. This means creating a JS wrapper for it. Currently it does not export all methods, they can be added in follow ups as needed.

Changelog

[General] [Added] - Export the DevSettings module, add addMenuItem method

Test Plan

Tested in an app using the following code.

if (__DEV__) {
 DevSettings.addMenuItem('Show Dev Screen', () => {
    dispatchNavigationAction(
      NavigationActions.navigate({
        routeName: 'dev',
      }),
    );
  });
}

Added an example in RN tester

janicduplessis · 2019-07-27T22:53:50Z

React/Modules/RCTDevSettings.mm

@@ -16,6 +16,8 @@
 #import "RCTProfile.h"
 #import "RCTUtils.h"

+#import <React/RCTDevMenu.h>


Is this okay? This header is from the DevSupport target. Otherwise I could check if bridge.devMenu exists at runtime.

cpojer · 2019-08-13T09:44:59Z

I like the idea of this but I'm not sure if this is a good idea. The dev menu should work when JS is not available. Adding options from JS means they won't be shown when reloading and Metro goes offline for example.

cc @rickhanlonii

janicduplessis · 2019-08-13T14:15:34Z

@cpojer This doesn’t really change the way the dev menu works, it simply allows to add addtional options from JS once the bundle is loaded. This is mostly useful in the context of a greenfield app.

The added menu options also still work when metro is offline as long as a js bundle is present.

janicduplessis · 2019-08-13T14:21:09Z

An example use case here is a Dev only screen in the context of a greenfield app (js screen + navigation) the option to open this screen should only be available when js is loaded anyway. Adding menu options from native is still supported for when it is needed, if the option need to be available before the js bundle is loaded.

janicduplessis · 2019-09-09T23:35:58Z

ping @cpojer :)

cpojer

Can you add the right flow types for NativeDevSettings?

cpojer

Ehh, meant to request changes…

cpojer · 2019-09-11T12:25:15Z

Libraries/Utilities/DevSettings.js

+
+    this._menuItems.set(title, handler);
+    this.addListener('didPressMenuItem', (event) => {
+      if (event.title === title) {


I don't feel good about using the string as identifier to connect them. Shall we use a numeric ID to keep track of them instead?

The reason for that is android already stores them by their name (https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java#L309) so it would break later anyway if 2 items have the same name. It also makes it easier to not add the same item multiple times when hot reloading (see this comment https://github.com/facebook/react-native/pull/25848/files#diff-bd79be22516654291770eb407afd366fR14).

Not ideal but in practice for a dev only module I thought this was fine.

Thanks, I hate it!

Do you mind rebasing and making the flow type change?

janicduplessis · 2019-09-13T18:22:00Z

@cpojer Done!

cpojer

Awesome!

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

cpojer · 2019-09-16T13:23:59Z

Your PR doesn't compile on Android and iOS. The Android error you can see on CI, the iOS one from fb internal:

React/Modules/RCTDevSettings.mm:338:10: error: unknown type name 'typeof'; did you mean 'typedef'?
  __weak typeof(self) weakSelf = self;

janicduplessis · 2019-09-16T18:13:11Z

@cpojer Oups, weird that the android one doesn't happen with gradle. Should be good now.

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

react-native-bot · 2019-09-17T13:40:06Z

This pull request was successfully merged by @janicduplessis in cc068b0.

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

rickhanlonii · 2019-12-26T14:39:23Z

@janicduplessis did this make it into the docs?

janicduplessis · 2019-12-26T15:21:58Z

@rickhanlonii I don’t remember doing it so probably not 😬

rickhanlonii · 2019-12-26T15:55:39Z

No sweat! Issue filed here to add it facebook/react-native-website#1523

mohamedsgap

Hey guys! @shergin @rickhanlonii @cpojer @janicduplessis
just finished writing doc for DevSettings module 'addMenuItem' as an API reference and need you to review PR to see if it seems good or need some changes!

slorber · 2020-02-14T09:11:38Z

@janicduplessis , thanks for this useful addMenuItem

Wonder if this does not make sense to provide a way to "remove" this menu item too?

This way, we could add contextual menu items.

useEffect(() => {
  if (__DEV__) {
    const remove = DevSettings.addMenuItem('Fill current form', () => {
       fillCurrentForm()
    });
    return remove;
  }
},[])

janicduplessis · 2020-02-14T09:25:58Z

@slorber Yea I think it makes sense, I kept the initial PR as simple as possible but it should be possible to implement this.

facebook-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. Contributor A React Native contributor. labels Jul 27, 2019

react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Jul 27, 2019

janicduplessis changed the title ~~Export DevMenu.addItem to JS~~ [WIP] Export DevMenu.addItem to JS Jul 27, 2019

janicduplessis requested a review from shergin as a code owner July 27, 2019 22:39

janicduplessis changed the title ~~[WIP] Export DevMenu.addItem to JS~~ [WIP] Export the DevSettings module, add addMenuItem method Jul 27, 2019

react-native-bot added the API: Settings label Jul 27, 2019

janicduplessis commented Jul 27, 2019

View reviewed changes

janicduplessis force-pushed the js-add-dev-menu-item branch from 7d90c01 to 7d0964c Compare July 28, 2019 15:54

janicduplessis changed the title ~~[WIP] Export the DevSettings module, add addMenuItem method~~ Export the DevSettings module, add addMenuItem method Jul 28, 2019

janicduplessis force-pushed the js-add-dev-menu-item branch from 7d0964c to cdabe7a Compare July 28, 2019 15:57

cpojer approved these changes Sep 11, 2019

View reviewed changes

cpojer suggested changes Sep 11, 2019

View reviewed changes

cpojer reviewed Sep 11, 2019

View reviewed changes

janicduplessis force-pushed the js-add-dev-menu-item branch 2 times, most recently from 228d9a9 to 55c6791 Compare September 13, 2019 18:20

janicduplessis force-pushed the js-add-dev-menu-item branch from ad87757 to 50cf797 Compare September 13, 2019 18:24

cpojer approved these changes Sep 16, 2019

View reviewed changes

facebook-github-bot approved these changes Sep 16, 2019

View reviewed changes

janicduplessis added 2 commits September 16, 2019 14:33

Export the DevSettings module, add addMenuItem method

5798cc1

Fix test

db54f02

janicduplessis force-pushed the js-add-dev-menu-item branch from d89fdfe to db54f02 Compare September 16, 2019 18:43

facebook-github-bot approved these changes Sep 17, 2019

View reviewed changes

facebook-github-bot closed this in cc068b0 Sep 17, 2019

react-native-bot added the Merged This PR has been merged. label Sep 17, 2019

rickhanlonii mentioned this pull request Dec 26, 2019

fix: debugger-ui name and reload button react-native-community/cli#844

Merged

rickhanlonii mentioned this pull request Dec 26, 2019

Add documentation for DevSettingsModule facebook/react-native-website#1523

Closed

mohamedsgap reviewed Jan 17, 2020

View reviewed changes

janicduplessis deleted the js-add-dev-menu-item branch January 17, 2020 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export the DevSettings module, add `addMenuItem` method #25848

Export the DevSettings module, add `addMenuItem` method #25848

janicduplessis commented Jul 27, 2019 •

edited

Loading

janicduplessis Jul 27, 2019

cpojer commented Aug 13, 2019

janicduplessis commented Aug 13, 2019

janicduplessis commented Aug 13, 2019

janicduplessis commented Sep 9, 2019

cpojer left a comment

cpojer left a comment

cpojer Sep 11, 2019

janicduplessis Sep 11, 2019 •

edited

Loading

cpojer Sep 12, 2019

janicduplessis commented Sep 13, 2019

cpojer left a comment

facebook-github-bot left a comment

cpojer commented Sep 16, 2019

janicduplessis commented Sep 16, 2019

facebook-github-bot left a comment

react-native-bot commented Sep 17, 2019

rickhanlonii commented Dec 26, 2019

janicduplessis commented Dec 26, 2019

rickhanlonii commented Dec 26, 2019 •

edited

Loading

mohamedsgap left a comment •

edited

Loading

slorber commented Feb 14, 2020 •

edited

Loading

janicduplessis commented Feb 14, 2020

Export the DevSettings module, add addMenuItem method #25848

Export the DevSettings module, add addMenuItem method #25848

Conversation

janicduplessis commented Jul 27, 2019 • edited Loading

Summary

Changelog

Test Plan

janicduplessis Jul 27, 2019

Choose a reason for hiding this comment

cpojer commented Aug 13, 2019

janicduplessis commented Aug 13, 2019

janicduplessis commented Aug 13, 2019

janicduplessis commented Sep 9, 2019

cpojer left a comment

Choose a reason for hiding this comment

cpojer left a comment

Choose a reason for hiding this comment

cpojer Sep 11, 2019

Choose a reason for hiding this comment

janicduplessis Sep 11, 2019 • edited Loading

Choose a reason for hiding this comment

cpojer Sep 12, 2019

Choose a reason for hiding this comment

janicduplessis commented Sep 13, 2019

cpojer left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

cpojer commented Sep 16, 2019

janicduplessis commented Sep 16, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

react-native-bot commented Sep 17, 2019

rickhanlonii commented Dec 26, 2019

janicduplessis commented Dec 26, 2019

rickhanlonii commented Dec 26, 2019 • edited Loading

mohamedsgap left a comment • edited Loading

Choose a reason for hiding this comment

slorber commented Feb 14, 2020 • edited Loading

janicduplessis commented Feb 14, 2020

Export the DevSettings module, add `addMenuItem` method #25848

Export the DevSettings module, add `addMenuItem` method #25848

janicduplessis commented Jul 27, 2019 •

edited

Loading

janicduplessis Sep 11, 2019 •

edited

Loading

rickhanlonii commented Dec 26, 2019 •

edited

Loading

mohamedsgap left a comment •

edited

Loading

slorber commented Feb 14, 2020 •

edited

Loading