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

[iOS] Fix "react_native_menu-Swift.h" cannot be found #807

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

alantoa
Copy link
Contributor

@alantoa alantoa commented May 11, 2024

Overview

CleanShot 2024-05-12 at 03 32 21@2x
I noticed that many users have been facing this build error on iOS since the last version was released. It is due to a change made in this PR.

I assume that this import is correct, but I'm not sure why @svbutko still encountered this issue. Could you please check if the build error is related to this? Thank you!

Test Plan

After changing it to #import <react_native_menu/react_native_menu-Swift.h>, the error disappeared.CleanShot 2024-05-12 at 03 38 08@2x

@alantoa alantoa requested a review from Naturalclar as a code owner May 11, 2024 19:38
@svbutko
Copy link
Contributor

svbutko commented May 11, 2024

Hey, @alantoa, the issue to which you are referring was happening before my PR the issue happens in NEW ARCH, while my fix fixes it for OLD ARCH.

You can check the version to which people are referring is 1.0.2 and what they are using

And that it happened more than 2 days before my PR was merged and released

Are you sure that you are using OLD ARCH?

@Darren120
Copy link

Hey, @alantoa, the issue to which you are referring was happening before my PR the issue happens in NEW ARCH, while my fix fixes it for OLD ARCH.

You can check the version to which people are referring is 1.0.2 and what they are using

And that it happened more than 2 days before my PR was merged and released

Are you sure that you are using OLD ARCH?

is it possible for expo to auto upgrade this package when doing npx expo upgrade --fix?

@alantoa
Copy link
Contributor Author

alantoa commented May 12, 2024

Yeah, that's kind of weird. Ideally, it should work for both versions, but only v1.0.2 works for me. Let me take another look then.

@Albert-Gao
Copy link

Yeah, that's kind of weird. Ideally, it should work for both versions, but only v1.0.2 works for me. Let me take another look then.

Wow, thanks, downgrade to v1.0.2 + Old Arch works now! Thanks! :)

"expo": "~51.0.8",
"react-native": "0.74.1",
"@react-native-menu/menu": "1.0.2",

@tompetk
Copy link

tompetk commented May 25, 2024

Any updates on this?

@sohail-wayland
Copy link

sohail-wayland commented May 28, 2024

This needs to be merged in as it's still a problem and it's causing build errors on iOS. Please merge this in asap @Naturalclar

@alantoa
Copy link
Contributor Author

alantoa commented May 28, 2024

@sohail-wayland, hey, this depends on @Naturalclar. I'm not quite sure which one is actually correct.
but for now, you can simply install v1.0.2, as its code is the same as this PR.

@awilson9
Copy link

+1 this is actively broken

@SohelIslamImran
Copy link

image Having same issue

@fobos531
Copy link
Contributor

Hey @Naturalclar - do you have the bandwidth to take a look at this?

Copy link
Member

@Naturalclar Naturalclar left a comment

Choose a reason for hiding this comment

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

LGTM

@Naturalclar Naturalclar merged commit cbc23fb into react-native-menu:master Jun 12, 2024
@thunderfalco
Copy link

image still have this error

@mrousavy
Copy link
Contributor

Hey! Thanks for this PR, but unfortunately for me it was the other way around - this PR broke the build now, while the previous release worked fine.

I think we should revert this, the previous way is the correct way of doing things (at least on latest Xcode).

Or maybe add an additional #if __has_include check to try both ways?

mrousavy added a commit to mrousavy/menu that referenced this pull request Jun 13, 2024
… old arch

The PR react-native-menu#807 broke the build for me on RN 0.74, latest Xcode / macOS.

Instead of just reverting it, I now check for both includes to see which one works, then use that.
@mrousavy
Copy link
Contributor

Created a fix for this here: #832 cc @Naturalclar

@alantoa
Copy link
Contributor Author

alantoa commented Jun 13, 2024

Thanks, Marc!
Yeah, adding the #if __has_include condition should work for both cases.

Naturalclar pushed a commit that referenced this pull request Jun 13, 2024
… old arch (#832)

The PR #807 broke the build for me on RN 0.74, latest Xcode / macOS.

Instead of just reverting it, I now check for both includes to see which one works, then use that.
@mrousavy
Copy link
Contributor

I'm curious why that works though. Why is it sometimes in a directory, and sometimes not?

@dancixx
Copy link

dancixx commented Jun 17, 2024

This error still comes up on the new arch. @alantoa

@alantoa
Copy link
Contributor Author

alantoa commented Jun 17, 2024

hey @dancixx what if you were to use this patch? #832

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.

None yet