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.

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

fix: more header path issues #121

Conversation

coolsoftwaretyler
Copy link
Contributor

@coolsoftwaretyler coolsoftwaretyler commented Dec 15, 2024

This PR closes #120 by updating the header paths to support both hyphens and underscores.

The first commit, 828bc0f, fixes it the way we've been writing them up until now.

In cd7840e, I wrote a macro to clean up those imports. Since we can't encapsulate #if statements in the preprocessor, it's not as clean as I'd like, but it does reduce all of the conditionals in each file.

You can see the first patch working in my reproducer repo, at this commit: coolsoftwaretyler/react-native-ios-utilities-missing-header-expo-52-reproducer@bfe92e1

You can see the second patch working in the reproducer at: coolsoftwaretyler/react-native-ios-utilities-missing-header-expo-52-reproducer@8dd3133

To verify:

  1. Pull down the reproducer
  2. rm -rf node_modules/ android/ ios/ for a clean start
  3. npm install
  4. npm run patch
  5. npx expo prebuild
  6. npm run ios

Build should work.

@coolsoftwaretyler coolsoftwaretyler force-pushed the fix/issue-120-expo-sdk-52-support branch from 8dbac1d to 3161150 Compare December 15, 2024 20:01
@coolsoftwaretyler coolsoftwaretyler force-pushed the fix/issue-120-expo-sdk-52-support branch from 3161150 to cd7840e Compare December 15, 2024 20:02
@coolsoftwaretyler
Copy link
Contributor Author

@dominicstop - will you take a look at this when you get a chance?

Let me know if you want me to update the example expo app to SDK 52. I wanted to at least try and patch up #120, but I can also update the example for you if that'll help in the long term as well.

Thanks in advance!

@dominicstop dominicstop merged commit 5adcc15 into dominicstop:master Dec 15, 2024
@dominicstop
Copy link
Owner

hello, @coolsoftwaretyler

thank you for continuing to contribute to this repo, and helping me catch bugs/errors; i have merged your changes and submitted a new release: v3.0.0-19

thank you for creating a repro; i cloned it, and installed the latest release:
Screenshot 2024-12-16 at 9 33 35 AM

Screenshot 2024-12-16 at 9 40 59 AM

there doesn't seem to be any problems, so maybe it's fixed now? i have closed the issue you mentioned, but please feel free to reopen if it not resolved yet.

if you have some free time, i would be thankful if you could update the expo example; for now, i'll use the repro you provided to validate future releases (i think it should be enough for basic tests).

@coolsoftwaretyler
Copy link
Contributor Author

coolsoftwaretyler commented Dec 16, 2024

There won't be issues with the repro anymore because I applied these patches there, so I think we're all good.

I'm about to be on a cross country move this week, but if I have some time I'll try to swing back and update the example expo project here as well.

If no one else gets around to it in the next few weeks, I'll at least get to it maybe early 2025.

Thank you!

@coolsoftwaretyler coolsoftwaretyler deleted the fix/issue-120-expo-sdk-52-support branch December 16, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants