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 subspec for iOS extensions without React #471

Merged
merged 2 commits into from
Jul 25, 2020

Conversation

Taylor123
Copy link
Contributor

@Taylor123 Taylor123 commented Jul 14, 2020

The current podspec lists React as a dependency. This causes RCTLinking to be built (among other deps).

This can be problematic when working on an iOS 14 Widget that needs to share config variables since WidgetKit, and other extensions, do not allow certain functionality used in RCTLinking.

Separating into subspecs gives the consumer of react-native-config the option to pick what dependencies are built by their pod target

@height
Copy link

height bot commented Jul 14, 2020

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@luancurti
Copy link
Collaborator

@Taylor123 Can you describe what problem are you fixing?

@Taylor123
Copy link
Contributor Author

@luancurti Updated the description! Let me know if you need more info

@luancurti
Copy link
Collaborator

@Taylor123 I have a question about this. If I install using pod 'react-native-config', :path => '../node_modules/react-native-config' everything is working as before? There isn't a breaking change?

@Taylor123
Copy link
Contributor Author

@luancurti from my understanding and what i've seen since using my branch for this, yes. The default subspec is set to App which will use the same source_files glob pattern as before and have the React dependency.

Though i just started seeing that the script_phase is not being inherited by the Extension subspec. Going to fix that locally and update the PR

@luancurti luancurti self-assigned this Jul 18, 2020
@luancurti
Copy link
Collaborator

@Taylor123 Ok, thanks for the explanation. I'll test this tomorrow and if everything is working I'll merge this PR. Thanks for contribute

@Taylor123
Copy link
Contributor Author

@luancurti Any chance you were able to test this out? Would love to get it released so I don't have to rely on my fork :)

@luancurti luancurti merged commit 38b87f9 into lugg:master Jul 25, 2020
@luancurti
Copy link
Collaborator

@Taylor123 published in npm version 1.3.2

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

Successfully merging this pull request may close these issues.

2 participants