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.

Sign up for GitHub

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

Jump to bottom

Android template: Allow overriding default "index.js" entry file #26769

Closed
wants to merge 1 commit into from
Closed

Android template: Allow overriding default "index.js" entry file #26769

wants to merge 1 commit into from

Conversation

rumax
Copy link
Contributor

@rumax rumax commented Oct 8, 2019 •
edited
Loading

Summary

  • Using "System.getenv" allows to specify any entry file using environment variables and without modifying gradle file. Example:

    export ENTRY_FILE="another_entry_file.js"
    ./gradlew assembleDebug

  • This functionality is also more align with iOS implementation that uses 'if [[ "$ENTRY_FILE" ]]; then'. See [iOS] [Added] - Allow overriding ENTRY_FILE on react-native-xcode.sh script #23667 for more details.

  • Possibility to define entry file on CI without modifying sources (Example: project like pixels-catcher requires different entry file)

Changelog

[Android] [Added] - Custom entry file on android using ENTRY_FILE environment variable

Test Plan

  • Create a project from template

  • Define ENTRY_FILE environment variable

export ENTRY_FILE="anotherIndexFile.js"
  • Build android
./gradlew assembleDebug

Expected result: App contains bundle file that starts from anotherIndexFile.js file.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 8, 2019
@react-native-bot react-native-bot added the Platform: Android Android applications. label Oct 8, 2019
@dulmandakh
Copy link
Contributor

I think it's best to do it in react.build file.

@rumax
Copy link
Contributor Author

rumax commented Oct 9, 2019 •
edited
Loading

I think it's best to do it in react.build file.

Good point, I'll update PR with this change.

@dulmandakh Updated.

@rumax rumax changed the title Android template: Allow overriding default "index.js" entry file WIP: Android template: Allow overriding default "index.js" entry file Oct 9, 2019
@rumax rumax changed the title WIP: Android template: Allow overriding default "index.js" entry file Android template: Allow overriding default "index.js" entry file Oct 9, 2019
Android template: Allow overriding default "index.js" entry file
5eff85b 
- Using "System.getenv" allows to specify any entry file using environment variables and without modifying gradle file. Example:

    export ENTRY_FILE="another_entry_file.js"
    ./gradlew assembleDebug

- This functionality is also more align with iOS implementation that uses 'if [[ "$ENTRY_FILE" ]]; then'. See #23667 for more details.
cpojer
cpojer approved these changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@cpojer cpojer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm not a big fan of this functionality if I'm being honest because we could just toggle the entry point within index.js using the Platform module without needing more build configuration.

Making it consistent with iOS makes sense to me, though. Thanks for the PR!

facebook-github-bot
facebook-github-bot approved these changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by Maksym Rusynyk in a0d8740.

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

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 14, 2019
@rumax rumax deleted the android_template_entry_file branch October 15, 2019 10:24
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 2, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
e6a4371 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 2, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
24bec42 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 4, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
8fc1401 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 9, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
942ee93 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 11, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
b0748fc 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 17, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
2404c2d 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 17, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
e0f5ba1 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 18, 2020
@chrisbobbe
android build: Omit entryFile, now that we can.
ed64638 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
gnprice pushed a commit to chrisbobbe/zulip-mobile that referenced this pull request Sep 18, 2020
@chrisbobbe @gnprice
android build: Omit entryFile, now that we can.
ab671e2 
Part of the RN v0.61 -> v0.62 changes to the template app [1],
corresponding to facebook/react-native@a0d874087. This must happen
at or after the main upgrade commit because the field is newly
optional at the new version.

There isn't a clear functional advantage to doing it this way. In
fact, the maintainer who merged it said he was "not a big fan" of
the change [2]. But he merged it anyway, on the grounds that it
makes the experience more consistent with iOS.

So, I suppose it does that for us, too, and in any case we're now
more consistent with the template app.

[1] https://react-native-community.github.io/upgrade-helper/?from=0.61.5&to=0.62.2
[2] facebook/react-native#26769 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot approved these changes

@cpojer cpojer cpojer approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rumax @dulmandakh @react-native-bot @cpojer @facebook-github-bot