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

Support for ES2018 Unicode character class escapes #1027

Open
1 task done
birdofpreyru opened this issue Jun 11, 2023 · 4 comments · May be fixed by #1295
Open
1 task done

Support for ES2018 Unicode character class escapes #1027

birdofpreyru opened this issue Jun 11, 2023 · 4 comments · May be fixed by #1295
Labels
enhancement New feature or request es2018 es2018+ compatibility

Comments

@birdofpreyru
Copy link

Bug Description

new RegExp(/\p{P}/, 'u') in JS code crashes React Native when using Hermes, throwing SyntaxError: Invalid RegExp: Invalid escape; and it works fine with JSC. \p{P} is the Unicode character class escape for punctuation.

  • I have run gradle clean and confirmed this bug does not occur with JSC

Hermes version: the one bundled with RN 0.71.10
React Native version (if any): 0.71.10
OS version (if any):
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64):

@birdofpreyru birdofpreyru added the bug Something isn't working label Jun 11, 2023
@tmikov tmikov added the es2018 es2018+ compatibility label Jun 11, 2023
@tmikov tmikov changed the title Regular expressions do not support unicode character class escape Support for ES2018 Unicode character class escapes Jun 11, 2023
@tmikov tmikov added the enhancement New feature or request label Jun 11, 2023
@neildhar neildhar removed the bug Something isn't working label Jun 20, 2023
@mandrade2
Copy link

Is this new-contributor friendly? I'd be happy to implement it. Need it in my app

@tmikov
Copy link
Contributor

tmikov commented Oct 26, 2023

@mandrade2 It's difficult to gauge exactly how new-contributor friendly it is. It requires good C++, understanding of Unicode, regular expressions in general, and the Hermes regex implementation. It is not strongly coupled to the rest of the runtime, but likely requires calling internal APIs in ways that they are already used in the internal implementation.

I would say that this is a moderately complex task that would require non-trivial time commitment.

@mandrade2
Copy link

Thanks @tmikov! I'll start looking around the Hermes repo to find out if I can do it. Will post any doubts here if that works out

@mandrade2
Copy link

@tmikov unfortunately I don't think I'll be available complete this. Hope I get more time to contribute in the future!

@jonathanj jonathanj linked a pull request Feb 3, 2024 that will close this issue
opiation added a commit to bonfhir/bonfhir 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
Labels
enhancement New feature or request es2018 es2018+ compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants