Skip to content

Commit

Permalink
Use remarkable for Markdown formatting to support React Native (#283)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
opiation committed May 1, 2024
2 parents 069044d + 3547a0f commit bf85c5d
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 29 deletions.
5 changes: 5 additions & 0 deletions .changeset/rich-ties-tell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@bonfhir/core": patch
---

Use `remarkable` instead of `marked` to format Markdown content. This avoids using regular expression syntax that isn't yet supported by the Hermes JS engine, allowing `@bonfhir/core` to load without error in Expo / React Native applications.
5 changes: 5 additions & 0 deletions .changeset/smart-elephants-tie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@bonfhir/react": patch
---

Only sanitize HTML output from Markdown formatting when formatted where a DOM is available (browser, Deno, etc.). Platforms without a DOM (React Native) have other means of rendering and sanitizing a given Markdown or HTML string into a relatively safe and usable UI.
16 changes: 8 additions & 8 deletions docs/website/docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,35 +164,35 @@ In order to achieve this:

:::info[Walkthrough]

Let's imagine that we need to add a dependency to the `marked` package to `@bonfhir/react`.
Let's imagine that we need to add the `remarkable` package as a dependency of `@bonfhir/react`.

1. Lookup existing information

```bash
pnpm packages:lookup marked
pnpm packages:lookup remarkable

> bonfhir@ packages:lookup bonfhir/bonfhir
> pnpm why -r "marked"
> pnpm why -r "remarkable"

Legend: production dependency, optional only, dev only

@bonfhir/core@2.17.2 bonfhir/bonfhir/packages/core
@bonfhir/core@2.19.2 bonfhir/bonfhir/packages/core

devDependencies:
marked 11.1.0
remarkable 2.0.1
```

2. Found it! It is used by the `@bonfhir/core` package. Let's look at the `package.json` file:

```json
{
"name": "@bonfhir/core",
"version": "2.17.2",
"version": "2.19.2",
"description": "Core FHIR resources and utilities for BonFHIR",
//...
"devDependencies": {
//...
"marked": "^11.1.0"
"remarkable": "^2.0.1"
//...
}
//...
Expand All @@ -202,7 +202,7 @@ marked 11.1.0
3. Install with the same version moniker:

```bash
pnpm add marked@^11.1.0
pnpm add remarkable@^2.0.1
```

:::
Expand Down
3 changes: 2 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,16 @@
"@rollup/plugin-typescript": "^11.1.6",
"@types/jest": "^29.5.12",
"@types/node": "^20.12.7",
"@types/remarkable": "^2.0.8",
"decimal.js": "^10.4.3",
"esbuild-jest": "^0.5.0",
"graphql": "^16.8.1",
"jest": "^29.7.0",
"jest-mock-extended": "^3.0.6",
"localized-address-format": "^1.3.1",
"marked": "^12.0.1",
"msw": "^2.2.13",
"nodemon": "^3.1.0",
"remarkable": "^2.0.1",
"rimraf": "^5.0.5",
"rollup": "^4.14.1",
"rollup-plugin-dts": "^6.1.0",
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/r4b/value-formatters/markdown.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { marked } from "marked";
import { Remarkable } from "remarkable";
import { ValueFormatter } from "../formatters";

let remarkable: Remarkable | undefined;
function htmlRenderer(opts?: Remarkable.Options): Remarkable {
if (!remarkable) remarkable = new Remarkable(opts);
return remarkable;
}

/**
* A FHIR string (see above) that may contain markdown syntax for optional processing
* by a markdown presentation engine
Expand All @@ -24,12 +30,13 @@ export const markdownFormatter: ValueFormatter<

switch (options?.style) {
case "bareString": {
return (marked.parse(value) as string)
return htmlRenderer()
.render(value)
.replaceAll(/<\/?[^>]+(>|$)/gi, "")
.trim();
}
case "html": {
return (marked.parse(value) as string).trim();
return htmlRenderer().render(value).trim();
}
// eslint-disable-next-line unicorn/no-null
case null:
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/r5/value-formatters/markdown.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { marked } from "marked";
import { Remarkable } from "remarkable";
import { ValueFormatter } from "../formatters";

let remarkable: Remarkable | undefined;
function htmlRenderer(opts?: Remarkable.Options): Remarkable {
if (!remarkable) remarkable = new Remarkable(opts);
return remarkable;
}

/**
* A FHIR string (see above) that may contain markdown syntax for optional processing
* by a markdown presentation engine
Expand All @@ -24,12 +30,13 @@ export const markdownFormatter: ValueFormatter<

switch (options?.style) {
case "bareString": {
return (marked.parse(value) as string)
return htmlRenderer()
.render(value)
.replaceAll(/<\/?[^>]+(>|$)/gi, "")
.trim();
}
case "html": {
return (marked.parse(value) as string).trim();
return htmlRenderer().render(value).trim();
}
// eslint-disable-next-line unicorn/no-null
case null:
Expand Down
12 changes: 11 additions & 1 deletion packages/react/src/r4b/data-display/fhir-value.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ export type DefaultFormatterParametersProps =
| ValueFormatterParametersAsProps<typeof valueFormatters.urlFormatter>
| ValueFormatterParametersAsProps<typeof valueFormatters.uuidFormatter>;

/**
* @note HTML generated from Markdown content with `style: "html"` will NOT be
* sanitized against XSS attacks in React Native environments. Consider using a
* different `style` or provide a renderer that handles the generated HTML
* carefully if this is a concern.
*/
export function FhirValue<TRendererProps = any>(
props: FhirValueProps<TRendererProps>,
): ReactElement<any, any> | null {
Expand All @@ -85,7 +91,11 @@ export function FhirValue<TRendererProps = any>(
props.options,
);

if (props.type === "markdown" && props.options?.style === "html") {
if (
props.type === "markdown" &&
props.options?.style === "html" &&
DOMPurify.isSupported
) {
formattedValue = DOMPurify.sanitize(formattedValue);
}

Expand Down
12 changes: 11 additions & 1 deletion packages/react/src/r5/data-display/fhir-value.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ export type DefaultFormatterParametersProps =
| ValueFormatterParametersAsProps<typeof valueFormatters.urlFormatter>
| ValueFormatterParametersAsProps<typeof valueFormatters.uuidFormatter>;

/**
* @note HTML generated from Markdown content with `style: "html"` will NOT be
* sanitized against XSS attacks in React Native environments. Consider using a
* different `style` or provide a renderer that handles the generated HTML
* carefully if this is a concern.
*/
export function FhirValue<TRendererProps = any>(
props: FhirValueProps<TRendererProps>,
): ReactElement<any, any> | null {
Expand All @@ -88,7 +94,11 @@ export function FhirValue<TRendererProps = any>(
props.options,
);

if (props.type === "markdown" && props.options?.style === "html") {
if (
props.type === "markdown" &&
props.options?.style === "html" &&
DOMPurify.isSupported
) {
formattedValue = DOMPurify.sanitize(formattedValue);
}

Expand Down
41 changes: 29 additions & 12 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit bf85c5d

Please sign in to comment.