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: react native expo failing builds #281

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

Conversation

ioannisj
Copy link
Contributor

@ioannisj ioannisj commented Jan 10, 2025

💡 Motivation and Context

Fix for build issues reported here:

https://posthog.com/questions/pods-suddenly-not-able-to-be-installed
https://posthog.com/questions/react-renderer-animations-primitives-h-file-not-found

Trying to understand what happened

Our latest iOS SDK version 3.19.0 has unfortunately borken builds for RN Expo users.
After investigating the issue, and trying to replicate it, I found that the issue was that RN Expo tools like to link the PostHog SDK statically.

My basic understanding of the underlying issue:
Because we were using #import "utils.h", the compiler searches for the file in the current directory first. If it is not found there, the compiler then searches all other include directories. This worked okay when PostHog was shipped as a framework/module since the search was limited within the module itself. However, when PostHog was shipped as a static library, the compiler would search and find matching header files in the whole project from other dependencies or RN expo libs.

  • In the case of primitives.h file not found error, the compiler was linking to react/rendered/animations/utils.h
  • In the case of src/dsp/dsp.h file not found error, the compiler was linking to another libwebp header file if that dependency was present in the project. (This becomes apparent when we look at the screenshot example of the error, we never #include "src/dsp/dsp.h" in our files).

Remedying the issue

To remedy this, I flattened the whole file structure of /vendor/libwebp (both to simplify things and to handle how pods in general try to change the project structure) and used #import "./utils.h" instead. My understanding is that the ./ explicitly tells the compiler to look for the file in the current directory and not extend the search to other include directories.

Reproducing the issue and the fix

I've attached a sample project that reproduces the issue and the fix with the following steps:

SamplePostHog.zip

Initial Setup

  1. Install dependencies:

    npm install
  2. Attempt initial iOS build:

    npx expo run:ios --no-build-cache --device

    This will fail with a 'primitives.h' file not found error - this is the first error we expect.

Add 'libwebp` dependency

  1. Edit ios/Podfile:

    target 'SamplePostHog' do
      pod 'libwebp'
      
      use_expo_modules!
  2. Install CocoaPods dependencies:

    pod install && cd ..
  3. Try building again:

    npx expo run:ios --no-build-cache --device

    This will fail with 'src/dsp/dsp.h' file not found error - this is the second error we expect.

Add local 'PostHog` dependency with the fix

  1. Update ios/Podfile again:

    target 'SamplePostHog' do
      pod 'libwebp'
      pod 'PostHog', :path => '../posthog-ios' # Local path to iOS SDK from this branch
    
      use_expo_modules!
  2. Reinstall CocoaPods dependencies:

    pod install && cd ..
  3. Build one final time:

    npx expo run:ios --no-build-cache --device

    The build should now complete successfully.

Notes

  • Make sure you're in the project root directory when running npm/expo commands

Additional Steps

  • I've renamed the internal framework we link in case of SPM from libwebp to phlibwebp to avoid conflicts in case libwebp is already present in the project. I haven't noticed any errors, but just playing it safe here.
  • I've modified Makefile to also build the PostHogExampleWithPods project as both a static library and dynamic framework.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@marandaneto unfortunately I couldn't verify this for Flutter, still need to fix my local env for that. I'll try and do it while you review this, but if it's easy for you to check it would be really helpful.

@ioannisj ioannisj requested a review from marandaneto as a code owner January 10, 2025 12:49
@marandaneto marandaneto requested a review from a team January 10, 2025 12:51
Copy link
Member

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants