Skip to content

Conversation

@maykonmichel
Copy link

Summary:

Fixes #41844.

Enabling source maps with SOURCEMAP_FILE in ios/.xcode.env caused issues for iOS projects with spaces in their names. The glitch originated from the react-native-xcode.sh script lacking double quotes around the $SOURCEMAP_FILE argument, leading to unintended word splitting.

I've patched this by modifying the script to enclose the $SOURCEMAP_FILE argument in double quotes, ensuring seamless source map generation, even for projects with spaces in their names.

Changelog:

[IOS] [Fixed] - double quote $SOURCEMAP_FILE to prevent globbing and word splitting

Test Plan:

  1. Initialize a new react-native project.
  2. Change the name of the iOS project to include spaces, such as "Reproducer App".
  3. Insert the line export SOURCEMAP_FILE="$DERIVED_FILE_DIR/main.jsbundle.map" into the ios/.xcode.env file.
  4. Compile the iOS application using Xcode on release mode.
  5. Examine the Xcode logs for the message "main.jsbundle.map: No such file or directory".
  6. Apply the simple change on this PR.
  7. Compile the iOS application using Xcode on release mode again.
  8. Examine the Xcode logs and there is no "main.jsbundle.map: No such file or directory" message anymore.

@facebook-github-bot 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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Dec 7, 2023
@analysis-bot
Copy link

analysis-bot commented Dec 7, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 20,316,104 +16,394
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 23,513,300 +16,393
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: d3e0430
Branch: main

@facebook-github-bot
Copy link
Contributor

@dmytrorykun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

maykonmichel added a commit to maykonmichel/reproducer-react-native that referenced this pull request Dec 18, 2023
@cipolleschi
Copy link
Contributor

/rebase - this comment will automatically rebase this PR on top of main.

@github-actions
Copy link

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jun 25, 2024
@github-actions
Copy link

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@maykonmichel maykonmichel deleted the bugfix/#41844 branch June 26, 2024 11:07
@maykonmichel
Copy link
Author

I wanted to follow up on this PR. I noticed that #42220, a similar fix, was merged, and I'm really happy to see that the issue has been resolved. However, I put a lot of effort into identifying the problem, creating reproducible steps, and writing a detailed fix for this. I was wondering if I could get some feedback on why my PR might have been overlooked and any tips on how I can ensure my contributions get more attention in the future.
I genuinely appreciate all the hard work the maintainers do and understand that managing such a large repo can be challenging. Any advice or insights would be really helpful!

cc: @cipolleschi @arushikesarwani94 @paulschreiber

@cipolleschi
Copy link
Contributor

@maykonmichel I apologize for what happened. Our usual policy is that, when two similar issues are open, the first one opened should be the one we merge.

In this specific case, I think that this was a mistake in good faith.
The timeline is something like this:

  • Your PR has been imported by @dmytrorykun.
  • There was another PR that was similar to yours, but not quite, that was merged by me. In an attempt not to try to land two times the same thing, @dmytrorykun abandoned yours internally.
  • Then your PR got lost (sadly - totally our fault)
  • The new PR got opened
  • @arushikesarwani94, not knowing about this one, imported the most recent one that got merged.

We are extremely thankful for the work you put into finding the root cause and suggesting a fix. We will try to be more mindful and careful about similar PRs.
One thing that we can do is to mention duplicates in PRs, when we spot them, so the two get linked by Github and we can go back to the oldest one. Community could definitely help with that!

I know it's not much, but I hope that the explanation helps.

@maykonmichel
Copy link
Author

@cipolleschi Thank you so much for the detailed explanation and for acknowledging what happened. I completely understand that managing a large project with many contributors can lead to such oversights, and I appreciate the transparency in your response. I'm still passionate about contributing to community and look forward to more opportunities to help out.

@cipolleschi
Copy link
Contributor

Thank you for your understanding. We really appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Stale There has been a lack of activity on this issue and it may be closed soon.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

main.jsbundle.map: No such file or directory when iOS project name includes spaces

4 participants