changes link file on Android to MainApplication.java for 0.29 update

[GantMan]

rnpm aka react-native link is broken with Android 0.29 - #8603

This gets it back to working again by checking for new MyApplication.java file, and curtailing the path when needed.

[ghost]

By analyzing the blame information on this pull request, we identified @grabbou to be a potential reviewer.

[GantMan]

note, the methodology for checking the file is the latest method: http://stackoverflow.com/questions/4482686/check-synchronously-if-file-directory-exists-in-node-js

[satya164]

cc @Kureev

[Kureev]

Hi @GantMan! Thanks for proposing this PR! I have a few small questions in order to process it faster :)
As I see, you're checking for a MainApplication.java and "roll back" to MainActivity.java in a case if it's not found. But taking into account that rnpm is now a part of react-native cli, I don't think we have any reasons to maintain previous versions. Does it make any sense?

[GantMan]

Hi @Kureev! My guess use case where someone would care is this: "I created project X in RN 0.28 and I created project Y in 0.29". I should be able to still work on my 0.28 project and even use react-native link to add new features to it. It shouldn't break functionality on my non-updated project. I might have good reason for not bringing it to 0.29 just yet.

[satya164]

May be support old versions for few releases and then drop the support.

[GantMan]

Yeah, ideal would be 3 phases.

1. this goes through as hotfix.
2. deprecate, and warn
3. remove

I'm happy to help with all 3 steps.

[Kureev]

@GantMan For your case with 0.28 & 0.29: if I understand correctly, app scaffolded by 0.28 will have a react-native@0.28 with current react-native link implementation under the hood. Same is fair for 0.29: it'll have a new 0.29 react-native with new link for MainApplication. The only controversial point is about react-native upgrade. Does it convert your MainActivity to MainApplication? If so, then I don't see a reason why we need prev. versions.

If 0.28 to 0.29 migration by using react-native upgrade wouldn't rename MainActivity to MainApplication, then we need to add it as a part of the migration (which is a desired behavior imo) or support N releases behind the last one (for react-native link).

[Kureev]

There should be no callback (it's Sync):

try {
  fs.accessSync(mainApplicationPath, fs.F_OK);
} catch (e) {
  //...error
}
//...success

[satya164]

if I understand correctly, app scaffolded by 0.28 will have a react-native@0.28 with current react-native link implementation under the hood

@Kureev is right. I forgot that the cli just forwards the commands to locally installed react-native. So we should not need check for older versions in the code.

[GantMan]

I will update accordingly, you'll see code soon :)

[GantMan]

@Kureev and @satya164 - Thanks for the feedback.

I wasn't sure what the userConfig variable should be, but I figured consistency with the internal name would be best. We don't want users setting userConfig.mainActivityPath and thinking that's correct.

I did a quick test, and upgrade from 0.28-> 0.29 does NOT create the application file. It's set as manual steps in the release: https://github.com/facebook/react-native/releases/tag/v0.29.0

[satya164]

@GantMan Can you also update the commit title to reflect the actual change?

[satya164]

@GantMan Yes, react-native upgrade doesn't work for 0.28 -> 0.29 and it'll probably be very complex to fix this since we're using yeoman for this.

[GantMan]

@satya164 - title is updated.

Hah yes, yeoman is quite tricky. Fortunately it's a separate concern. I was just checking for @Kureev as he seemed to inquire specifically on this upgrade path. Let me know if there's more on this ticket that needs attention.

[Kureev]

I don't think there is anything else. Seems you covered everything, @GantMan 😉

[Kureev]

So, I'm going to check it on my local machine today and if everything is fine, I'm about to merge it

[grabbou]

Great work! I like it. Another thing is that we remove mainActivityPath now, which could potentially break plugins, but since there's no need to use main activity anymore on the Android side, I think it's fine.

[Kureev]

@facebook-github-bot shipit

[ghost]

Thanks for importing. If you are an FB employee go to Phabricator to review.

[grabbou]

@mkonicek shall we release a 0.29.1 and also cherry-pick that into 0.30rc? @Kureev

[satya164]

@grabbou There was a bug in InteractionManager which we should also include. cc @sahrens

[grabbou]

@satya164 into the 0.29 or 0.30 branch?

[satya164]

@grabbou Both

[grabbou]

Ping me on Messagner later just in case

[satya164]

@grabbou Will do. Waiting for @sahrens to push it into OSS. He said the fix is up internally.

[sahrens]

Sorry, still waiting on review. If you need to patch locally, the fix is a one-liner, just change the filter call in TaskQueue#cancelTasks from .filter((queue) => queue.tasks.length > 0); to:

.filter((queue, idx) => (queue.tasks.length > 0 || idx === 0));

[Kureev]

We missed one change here! Since we renamed mainActivityPath to mainFilePath, it should be also changed in the userConfig object. Want to hotfix, @GantMan?

[Kureev]

Seems this is a real scope of the change: https://github.com/facebook/react-native/search?utf8=%E2%9C%93&q=mainActivityPath. Hopefully, it's just about

find . -name '*.js' -type f -exec sed -i '' 's/mainActivityPath/mainFilePath/g' {} +

[Kureev]

see referenced PR for more info.

[rclai]

Glad to know I wasn't insane. When are you guys going to finish fixing this?

[GantMan]

Looks good @Kureev.

@rclai - look at PR #8807

Let me know if I can help any further.