Better error message when missing entry file #29012
Closed
petrbela wants to merge 2 commits into
Closed
Conversation
If the entry file has been renamed (e.g., to `index.ts`), the script currently fails silently and prints out that the bundle file does not exist. This change will print the correct error message instead.
facebook-github-bot
added
the
CLA Signed
Contributor
Author
|
The failing tests seem to be unrelated... Is there anything I need to do about them? |
Contributor
|
You can rebase to latest master and see if that helps. But anyway, now is the moment we wait for a Facebook employee to import it and verify |
kiranjd
reviewed
Jun 16, 2020
| fi | ||
|
|
||
| if [[ $DEV != true && ! -f "$ENTRY_FILE" ]]; then | ||
| echo "error: Entry file $ENTRY_FILE does not exist. If you use another file as your entry point, pass ENTRY_FILE=myindex.js" >&2 |
There was a problem hiding this comment.
I think it's a typo.
It should have been: "Error: entry file...
Contributor
Author
There was a problem hiding this comment.
Not sure what should be correct but error: File ... case is already used towards the end of this file so I followed the same pattern.
|
I think you don't need to capitalize the word after colon(:)
Kiran JD
…On Tue, Jun 16, 2020 at 7:47 AM Petr Bela ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/react-native-xcode.sh
<#29012 (comment)>
:
> @@ -67,9 +67,14 @@ if [[ "$ENTRY_FILE" ]]; then
# Use ENTRY_FILE defined by user
:
elif [[ -s "index.ios.js" ]]; then
- ENTRY_FILE=${1:-index.ios.js}
- else
- ENTRY_FILE=${1:-index.js}
+ ENTRY_FILE=${1:-index.ios.js}
+else
+ ENTRY_FILE=${1:-index.js}
+fi
+
+if [[ $DEV != true && ! -f "$ENTRY_FILE" ]]; then
+ echo "error: Entry file $ENTRY_FILE does not exist. If you use another file as your entry point, pass ENTRY_FILE=myindex.js" >&2
Not sure what should be correct but error: File ... case is already used
towards the end of this file so I followed the same pattern.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#29012 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AGFANA2RLOIWQI77UQ4UPM3RW3I23ANCNFSM4NO6GICQ>
.
|
Contributor
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@TheSavior has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Contributor
|
@TheSavior merged this pull request in e73208e. |
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Jul 29, 2020
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Oct 8, 2020
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Oct 21, 2020
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Oct 31, 2020
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Nov 23, 2020
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Jan 24, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Feb 18, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Mar 14, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Apr 29, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
May 2, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
May 12, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
May 13, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Jun 4, 2021
This reverts commit e73208e.
facebook-github-bot
pushed a commit
that referenced
this pull request
Sep 1, 2021
Summary: The original $ENTRY_FILE check was added in #29012 to help catch misconfiguration for the entry JS file. That turned out breaking some RNTester builds/tests, so #29263 was added to accommodate the fact that RNTester .xcodeproj file has its own directory hierarchy. The 2nd PR had multiple issues: * It is incorrect to assume that the $ENTRY_FILE always exists in the parent dir of the .xcodeproj location. This caused an issue in RC 0.66: react-native-community/releases#249 (comment) * RNTester has since moved to packages/rn-tester/ (from RNTester/), hence breaking that assumption It turns out RNTester .xcodeproj has incorrectly misconfigured this JS bundling step (not sure since when). The original script invocation passed in the correct path for `RNTesterApp.ios.js`, but as an arg to the `react-native-xcode.sh` instead of by setting `ENTRY_FILE` env var. So this diff does 2 things: * Undid #29263 * Fix RNTester JS bundling invocation to set the ENTRY_FILE correctly {F659123377} Changelog: [iOS][Fixed] Unbreak $ENTRY_FILE handling for JS bundling Reviewed By: lunaleaps Differential Revision: D30690900 fbshipit-source-id: 7c5802b3eac56c0456edcd4b7478bfa4af48fc27
lunaleaps
pushed a commit
that referenced
this pull request
Sep 1, 2021
Summary: The original $ENTRY_FILE check was added in #29012 to help catch misconfiguration for the entry JS file. That turned out breaking some RNTester builds/tests, so #29263 was added to accommodate the fact that RNTester .xcodeproj file has its own directory hierarchy. The 2nd PR had multiple issues: * It is incorrect to assume that the $ENTRY_FILE always exists in the parent dir of the .xcodeproj location. This caused an issue in RC 0.66: react-native-community/releases#249 (comment) * RNTester has since moved to packages/rn-tester/ (from RNTester/), hence breaking that assumption It turns out RNTester .xcodeproj has incorrectly misconfigured this JS bundling step (not sure since when). The original script invocation passed in the correct path for `RNTesterApp.ios.js`, but as an arg to the `react-native-xcode.sh` instead of by setting `ENTRY_FILE` env var. So this diff does 2 things: * Undid #29263 * Fix RNTester JS bundling invocation to set the ENTRY_FILE correctly {F659123377} Changelog: [iOS][Fixed] Unbreak $ENTRY_FILE handling for JS bundling Reviewed By: lunaleaps Differential Revision: D30690900 fbshipit-source-id: 7c5802b3eac56c0456edcd4b7478bfa4af48fc27
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Nov 8, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Nov 15, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Nov 25, 2021
This reverts commit e73208e.
janicduplessis
added a commit
to janicduplessis/react-native
that referenced
this pull request
Nov 30, 2021
This reverts commit e73208e.
facebook-github-bot
pushed a commit
that referenced
this pull request
Jun 10, 2022
Summary: Reverts #29012 It is not really possible to properly validate if ENTRY_FILE exists since it is resolved by metro, and the server is not always running from the app root, like in a monorepo with multiple RN apps running on the same metro server. In my case I run metro on the repo root and entry file will be something like `apps/app/index.js`. `-f "$ENTRY_FILE"` will fail since the script is run from the project folder (`PROJECT_ROOT`, in my case the `apps/app` folder, so it tries to resolve `apps/app/apps/app/index.js`). I don't think this check is actually useful since metro will report the error if the entry file is invalid (fixed in #30150). The error is not as user friendly, but I think it is still fine. Maybe it could be improved in metro. ## Changelog [iOS] [Fixed] - Don't validate ENTRY_FILE in react-native-xcode.sh Pull Request resolved: #32762 Test Plan: Before <img width="854" alt="image" src="https://user-images.githubusercontent.com/2677334/146100834-39310c9f-1767-496a-8698-1026726a1120.png"> After if file is actually missing <img width="987" alt="image" src="https://user-images.githubusercontent.com/2677334/146100893-d01e2247-c787-4174-ac60-2f308c338c8f.png"> After if file exists, builds successfully. Reviewed By: cortinico Differential Revision: D37066073 Pulled By: cipolleschi fbshipit-source-id: 8f6b149099a39d9179996bb965daa6cd9e06feac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
If the entry file has been renamed (e.g., to
index.ts), the script currently fails silently and prints out that the bundle file does not exist.This change will print the correct error message instead.
Changelog
[iOS] [Fixed] - Better error message when missing entry file
Test Plan
N/A