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

Add a few import required for the React Native wrapper distribution as iOS XCFramework #50009

Merged
merged 4 commits into from
May 4, 2023

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Apr 22, 2023

What? Why? and How?

Adds a few imports in a few react-native-bridge/ios code. These are required for the setup to compile, as can be seen in wordpress-mobile/gutenberg-mobile#5695

Testing Instructions

This PR should not affect the plugin behavior as it only touches files in the React Native brige package. As far as I understand the setup, green CI in the [gutenberg-mobile PR using these changes]wordpress-mobile/gutenberg-mobile#5695) should be enough to establish correctness.

Testing Instructions for Keyboard

N.A.


Ping @fluiddot @geriux can you help me get this reviewed and merged? I don't have the rights to ask for a review. Thanks!

Without this, when using the experimental static linking which should
hopefully enable us to wrap Gutenberg mobile in an XCFramework to import
in iOS projects, we'd get a build failure with `WKUserScript` not found.
This allows the compiler to find the `RCTLogLevel` type used in the
file.
This allows the compiler to find the `RCTEventEmitter` type which is
used in the file.
@mokagio mokagio marked this pull request as ready for review May 1, 2023 06:58
@geriux geriux self-requested a review May 3, 2023 09:59
Copy link
Member

@geriux geriux left a comment

Choose a reason for hiding this comment

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

LGTM! CI checks passed and I also tested it locally using the demo app and also the main apps 🚀

@geriux
Copy link
Member

geriux commented May 3, 2023

@mokagio Let me know if you want me to merge it, not sure if you have rights to merge. Thanks!

@mokagio
Copy link
Contributor Author

mokagio commented May 4, 2023

Thanks @geriux ! 🙌

Yes, I'd like this to be merged at your earliest convenience. Once it lands, it will allow me to stop pointing to a different submodule commit and use whatever is set in trunk 😄

@geriux geriux merged commit 1db3fd7 into WordPress:trunk May 4, 2023
@geriux geriux added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label May 4, 2023
@github-actions github-actions bot added this to the Gutenberg 15.8 milestone May 4, 2023
@geriux
Copy link
Member

geriux commented May 4, 2023

Merged! By the way it looks like you have enough contributions merged to be able to request contributor access 🎉 you can find more info about it here, more specifically:

If you meet this criterion of several meaningful contributions having been accepted into the repository and would like to be added to the Gutenberg team, feel free to ask in the #core-editor Slack channel.

@mokagio mokagio deleted the mokagio/add-ios-xcframework-imports branch May 5, 2023 04:09
@mokagio
Copy link
Contributor Author

mokagio commented May 5, 2023

Thanks @geriux 🎉

@bph bph added Mobile App - Release PRs for native mobile editor releases and removed Mobile App - Release PRs for native mobile editor releases labels May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants