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

chore(apple): fix build issues caught internally #2629

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Saadnajmi
Copy link
Contributor

Summary

At my side of Microsoft, we have recently "vendored" a couple of libraries. Which means, we started building them from source using our own generated Xcode project (not the one cocoa pods generates), which has much stricter build flags / treats warnings as errors. In doing so, I caught a couple of issues in React Native SVG I'd like to push back up!

Namely:

  • Fix a bunch of errors where "declaration of local variable shadows a previous declaration" by renaming some variables
  • Add __unused clang pragmas in a couple of places
  • Move some enums from CF_ENUM to NS_ENUM, and include an import of <Foundation.h>
  • cast some NSIntegers to intss or longs depending on context
  • Fix some headers by changing them from Header Search Paths (<Foo.h>) to User Header Search Paths ("foo.h")
  • Rename "self" to "defaults" inside RNSVGFontData to fix a "Declaration shadows a local variable" (I guess self is always in scope?)
  • Import <TargetConditionals.h> in places where we use #if TARGET_OS_OSX ifdefs
  • Also ifdef out some iOS files on macOS, and vice versa. Specifically, RNSVGTopAlignedLabel
  • In RNSVGSpan, move the label object from declared in a random spot in the file (??!) to be a proper ivar... this one was fun to catch.
  • Import RNSVGUIKit in one spott where it's used but not referenced.
  • Move the union kCGImageAlphaPremultipliedLast | kCGBitmapByteOrder32Big; to a separate variable. Technically, those two types are actually values from two different enums, but they are meant to be used together and cast, so 🤷‍♂️

Test Plan

CI should pass at the very least

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS ✅❌
MacOS ✅❌
Android ✅❌
Web ✅❌

Checklist

  • I have tested this on a device and a simulator
  • I added documentation in README.md
  • I updated the typed files (typescript)
  • I added a test for the API in the __tests__ folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant