Make @bonfhir/core compatible with react-native / hermes
#139
Assignees
Labels
bug
Something isn't working
Comments
|
I have explored a bit the problems. Proeminent problems:
We will need at least those 2 features to be implemented for this to make sense. |
|
Likely good news:
On a somewhat related side note though, it seems strange for to require class support in Hermes in an Expo app when Expo can already transpile JS. Maybe I'm missing something but is this because Expo doesn't transpile down classes or doesn't transpile dependencies? In the meantime, I'm going to test out using bonfhir with the latest Expo / React Native to see what's actually still needed and/or still failing. |
|
When using the I can't validate if the |
|
The regular expression syntax causing the first error reported in the issue description come from I was however able to replace I'll dig a little deeper to confirm this is actually a viable solution. If I can get an Expo app working on my Android device and actually formatting some Bonfhir-fetched Markdown from a dummy EHR, I'll submit a PR to replace @julienblin, any objections to such a change if this actually gets Bonfhir working on the Hermes engine? |
|
@opiation - great job and investigation. If it gets React Native Android to work, by all means make the switch! |
|
I can confirm getting Markdown formatting work with the following snippet on my Android device in Expo Go: import { Formatter } from "@bonfhir/core/r4b";
import { Text } from "react-native";
const formatter = Formatter.default;
const exampleMarkdown = "# Footer\n\nThis is a footer";
export default function RootLayout() {
return <Text>{Formatter.default.format("markdown", exampleMarkdown)}</Text>
}
This confirms that |
|
I'm now facing a new issue that we could not reach before with FWIW, I've gotten this issue before in a previous Expo project without Bonfhir. If memory serves, DOMPurify's implementation is a factory function that returns an object with a bunch of methods including I'm going to try a little defensive coding by checking for a |
|
I found out why DOMPurify is not working for React Native. It's not supported 🤦. This early return when creating the DOMPurify instance is taking effect in React Native environments which is why I think this makes a lot of sense because React Native doesn't support any HTML by itself. It requires using a WebView or similar tooling to turn HTML strings into React Native UI. I'll explore some React Native solutions to rendering HTML strings. I suspect that whatever solution we find will require conditionally using |
|
@julienblin, I propose the following for supporting React Native with
If someone wants to render Markdown into a React Native application, they might consider rendering Markdown directly with something like react-native-markdown-display or use react-native-webview to render the formatted HTML. I'm assuming this a reasonable compromise to get better React Native compatibility here so I'm going to include this slight change in the PR migrating from |
|
So, I think #283 should address the errors reported in the issue description. Specifically, the switch to
The Hermes team addressed this one by implementing ES classes in facebook/hermes#685.
The issue title however is about Hermes doesn't support |
opiation
added a commit
that referenced
this issue
May 1, 2024
Contributes to #139 This PR uses the [`remarkable`][remarkable] library to transform Markdown into HTML instead of `marked`. While `marked` is a competent library, it uses regular expressions to parse Markdown which include a [syntax currently unsupported by the Hermes JS engine](facebook/hermes#1027). In contrast, `remarkable` does use any regular expressions to parse so it seems to work just fine in React Native. After switching to `remarkable`, using `<FhirValue>` in an Expo application to display Markdown content in HTML resulted in a new error. This turned to be a straightforward issue that the `dompurify` library cannot sanitize an HTML string with access to the DOM. Thus, this PR narrows HTML sanitizing to only be done when `dompurify` can do so. ## How can I test this change? Create a new Expo app using the bonfhir template for it. Add a `<FhirValue>` with some example Markdown content styled as HTML and validate that the HTML shows up literally on the Android screen. ```tsx const exampleMarkdown = "# Footer\n\nThis is a footer"; function ExampleScreen() { return <FhirValue options={{ style: "html" }} type="markdown" value={exampleMarkdown} /> } ``` The above should show something like: ```html <h1>Footer</h1> <p>This is a footer</p> ``` ## Caveat This helps make `@bonfhir/core` and `@bonfhir/react` more compatible with React Native configured with Hermes by addressing specific issues. Full Hermes compatibility is not confirmed yet however as testing the complete surface of Bonfhir libraries and APIs is out of scope of this fix. We welcome the community finding more compatibility issues however that we can prioritize and address as needed! [remarkable]: https://github.com/jonschlinkert/remarkable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Here is the list of issues spit out by hermes compilation:
It seems they all come from the
markedpackage.Also, this: markedjs/marked#2843
The text was updated successfully, but these errors were encountered: