iamolegga / react-native-launch-arguments

Get launch arguments for testing with Detox and Appium
MIT License
55 stars 21 forks source link

Empty set of launch arguments if used in a very early stage (Android) #44

Closed d4vidi closed 1 year ago

d4vidi commented 2 years ago

Hi, Detox maintainer here πŸ‘‹πŸ» I've recently been making usage of this library (great job, BTW!) in Detox's self-test project. We see that quite clearly, on Android at least, if the launch arguments are used in a very early stage (e.g. in the app's index.js), then intermittently, an empty arguments object is returned.

For example:

import React, {Component} from 'react';
import {
  AppRegistry,
} from 'react-native';
import { LaunchArguments } from 'react-native-launch-arguments';

console.warn('DEBUG', LaunchArguments.value());

export default class ExampleApp extends Component {
// ...
}

AppRegistry.registerComponent('example', () => ExampleApp);

If the app is repeatedly launched using device.launchApp({ launchArgs: { 'my': 'arg' } }), then you would see logs looking somewhere along the lines of:

DEBUG { my: arg }
DEBUG { my: arg }
DEBUG {}
DEBUG { my: arg }
DEBUG { my: arg }
DEBUG { my: arg }
DEBUG {}

Side note: I suppose there's a race condition here, sorting out the arguments into the NativeModule's constants.

d4vidi commented 2 years ago

@iamolegga do you think there's a way to get this sorted out somehow? Perhaps Detox's implementation could provide a decent alternative, although asynchronous.

iamolegga commented 2 years ago

@d4vidi sorry, I'm not able to investigate this in the nearest future. But I would be glad to merge fix for this. Maybe in the late 2022 I'll get back to this issue (and all the others) if on my current project we will use this lib

d4vidi commented 2 years ago

I could provide a fix but best if you give a general approval πŸ˜„

iamolegga commented 2 years ago

PR's are always welcome πŸ™‚

d4vidi commented 2 years ago

Perhaps Detox's implementation could provide a decent alternative, although asynchronous.

I meant, an approval of this, in particular.

iamolegga commented 2 years ago

So you want to replace built-in method with the detox's one? Then we need to check that:

Does getLaunchArguments mentioned in the detox's docs? If this is available for the end user then this library can be deprecated in favor of detox built-in method, right?

d4vidi commented 2 years ago

It is not a Detox top-level API - Detox never runs inside the app. The Detox code I pointed at was of Detox's self-test app.

d4vidi commented 2 years ago

The main question is whether you'd be willing to change the API in a breaking way, namely from:

import {LaunchArguments} from 'react-native-launch-arguments';
const args = LaunchArguments.values()

to something along the lines of:

import {  NativeModules } from 'react-native';
const { LaunchArguments } = NativeModules;
const args = await LaunchArguments.values();
iamolegga commented 2 years ago

I believe we can do this inside index.js of this module:

import {  NativeModules } from 'react-native';
const { LaunchArguments } = NativeModules;
export { LaunchArguments }

then only asynchrony will changed, and from my point of view that's ok if the bug can be fixed only this way

d4vidi commented 2 years ago

Cool

richardruiter commented 1 year ago

I believe we can do this inside index.js of this module:

import {  NativeModules } from 'react-native';
const { LaunchArguments } = NativeModules;
export { LaunchArguments }

then only asynchrony will changed, and from my point of view that's ok if the bug can be fixed only this way

Is this fixed? We have this problem as well that is fails on Android.

PinhoL commented 1 year ago

Been happening to me as well, Android only

d4vidi commented 1 year ago

Now actively trying to get this fixed

d4vidi commented 1 year ago

Still researching. This is what I've done so far: https://github.com/wix/Detox/pull/4060

d4vidi commented 1 year ago

Here are the full details: https://github.com/facebook/react-native/issues/37518

iamolegga commented 1 year ago

Should be fixed in #72 by @d4vidi

Please try npm i react-native-launch-arguments@next and comment here if it works correctly, I'll publish it as a new major release then.

d4vidi commented 1 year ago

Should be fixed in #72 by @d4vidi

Please try npm i react-native-launch-arguments@next and comment here if it works correctly, I'll publish it as a new major release then.

Great, thank you. I'll integrate that back into the Detox self-test app, we'll see how it goes.

d4vidi commented 1 year ago

Seems like the problem hasn't been resolved, according to Detox: There is a still another subtle update required: #73.

iamolegga commented 1 year ago

@d4vidi I've just published 4.0.0-rc.1. Please try again npm i react-native-launch-arguments@next

d4vidi commented 1 year ago

Thanks!

d4vidi commented 1 year ago

@iamolegga things are looking stable on the Detox side (https://github.com/wix/Detox/pull/4060) πŸ‘πŸ» LMK when you have an official release so we can get off the next tag

iamolegga commented 1 year ago

@d4vidi done βœ