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

[5.x] Made WebView an optional dependency which can be passed as a prop to <HTML> to support iframes #301

Closed

Conversation

IjzerenHein
Copy link
Contributor

Hi 👋

This PR makes the WebView component optional, so you can choose to include it or not. I'll try to explain my reasoning about why WebView is preferably optional for this package.

  • As of RN60s auto-linking feature, any native dependencies are automatically linked into your project, even if you aren't using them. This was true for the react-native-webview component, which is installed when installing react-native-render-html. Linking packages unnecessary creates a problem as it often requires additional OS permissions or usage strings. It may cause your deploy to the App or Play store to fail. And obviously it also increases your binary size.
  • react-native-render-html is a great way to render rich-content without requiring something heavy like a webview. It is this aspect which makes this library so damn useful and appealing.
  • Because of the dependency on react-native-webview, this project becomes less compatible with older react-native versions, as well as requires more maintenance to keep the webview dependency up to date (there is already 4 PRs concerning webview-package updates).

I therefore suggest an opt-in approach, where people that are using iframes can specify the WebView component that shall be used. The iframe-renderer then takes the WebView component and renders the content. When no WebView is specified to <HTML>, but a iframe is renderered nonetheless, a warning is written to the console (Unable to render <iframe>, please specify the WebView prop in <HTML>).

For the latest react-native versions you would then use it like this (when you need iframes):

import { WebView } from 'react-native-webview`;

export default () => (
  <HTML ... WebView={WebView} />
);

And for older react-native versions you import WebView from react-native itself:

import { WebView } from 'react-native`;

And obviously, if you don't use iframes, then just leave it out:

export default () => (
  <HTML ... />
);

Cheers and thank you for this library!

@vomchik

This comment has been minimized.

@jsamr jsamr added the pr:review needed This PR requires review from maintainers. label Jul 5, 2020
@jsamr
Copy link
Collaborator

jsamr commented Jul 9, 2020

@IjzerenHein Thank you for this contribution, and sorry for the huge delay. I am a new maintainer, and I want to make this project live again. I would tend to agree with you; this is the “dependency injection” approach I adopted in react-native-render-html-table-bridge. Because this is a breaking change, it will not be merged soon, but we will certainly consider it in a future major release.

@jsamr jsamr added pr:breaking Pull Request introducing a breaking change. and removed pr:review needed This PR requires review from maintainers. labels Jul 9, 2020
@jsamr jsamr changed the title Made WebView an optional dependency which can be passed as a prop to <HTML> to support iframes [5.x] Made WebView an optional dependency which can be passed as a prop to <HTML> to support iframes Nov 27, 2020
@jsamr
Copy link
Collaborator

jsamr commented Dec 5, 2020

This feature has been incorporated in the new v5.0.0 release! Thanks for your contribution. See #442

@jsamr jsamr closed this Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking Pull Request introducing a breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants