react-native-live-markdown icon indicating copy to clipboard operation
react-native-live-markdown copied to clipboard

Support custom JS parsers without having separate bundle

Open tomekzaw opened this issue 1 year ago • 11 comments

Currently, Live Markdown comes with a JS parser/formatter that wraps ExpensiMark and converts Markdown message into a list of ranges (style, location, length). This code needs to be run on the UI runtime while typing so it is executed on a separate JS runtime. Currently, this code is distributed as a separate bundle which is loaded from this file distributed along with the Live Markdown library: https://github.com/Expensify/react-native-live-markdown/blob/main/parser/react-native-live-markdown-parser.js

However, this makes things more difficult to maintain since after each change in expensify-common we need to bump it in @expensify/react-native-live-markdown and then bump both these libraries in Expensify/App repo. This also leads to inconsistencies when E/App and Live Markdown use different versions of expensify-common.

Finally, since the JS parser/formatter is bundled, there's no way to parametrize its behavior based on app state, however in some cases this will be necessary, e.g. to implement:

  • short mentions (we need to pass a list of previously mentioned users)
  • correct background color for user mentions when the current user is mentioned (should be green instead of blue)

The objective of this task is to allow passing JS parser/formatter as a JS function directly to MarkdownTextInput component. Here's the current idea for the API:

function App() {
  const parser = useMarkdownParser((markdown: string) => {
    'worklet';
    // some code here
    return [
      {type: 'syntax', start: 2, length: 1},
      {type: 'bold', start: 3, length: 5},
      {type: 'syntax', start: 8, length: 1},
    ];
  }, []);

  return <MarkdownTextInput
        multiline
        autoCapitalize="none"
        value={value}
        onChangeText={setValue}
        style={styles.input}
        ref={ref}
        markdownStyle={markdownStyle}
        parser={parser}
        placeholder="Type here..."
        onSelectionChange={(e) => setSelection(e.nativeEvent.selection)}
        selection={selection}
      />;
}

The current implementation of JS parser/formatter will be exported as the following hook:

const parser = useExpensiMarkParser();

tomekzaw avatar Apr 16 '24 23:04 tomekzaw

Problem/Solution: https://swmansion.slack.com/archives/C01GTK53T8Q/p1716235671381059

tomekzaw avatar May 20 '24 20:05 tomekzaw

@tomekzaw do you have the bandwidth to prioritize this? If not, wanna see about tag-teaming it with another engineer or finding a volunteer to take over? I know you posted the P/S just yesterday, there was a month+ before that without an update, which is why I'm asking.

mallenexpensify avatar May 22 '24 18:05 mallenexpensify

@mallenexpensify Actually, I have a working proof of concept that uses Reanimated worklets (Reanimated is already a dependency so that's cool) so we might be able to use that quite soon. The only problem for the time being is that ExpensiMark still uses jQuery in some places, but this is something that can be parallelized or even made external, what do you think?

tomekzaw avatar May 22 '24 19:05 tomekzaw

@tomekzaw , honestly, I have no clue. I'm not familiar with this. Looks like a plan was hatched in this thread and that https://github.com/Expensify/App/issues/42494, linked above, is the next step. thx.

mallenexpensify avatar May 23 '24 19:05 mallenexpensify

Working on it, looks promising, will share a video recording with a demo early next week.

tomekzaw avatar Jun 28 '24 09:06 tomekzaw

@tomekzaw Is there any update here?

shubham1206agra avatar Jul 18 '24 06:07 shubham1206agra

Latest update here

tomekzaw avatar Jul 18 '24 06:07 tomekzaw

@tomekzaw can you write a short update here for open source users without access to SM Slack?

szado avatar Jul 18 '24 08:07 szado

Sure, will do.

TL;DR in an upcoming version of react-native-live-markdown, MarkdownTextInput component will expose a parser prop that accepts a JS function (must be a worklet, since it's run on the UI thread as you type) and returns an array of ranges (like {start: 2, length: 4, type: 'link'}). This way it will be possible to implement custom formatting logic full in JS with fast-refresh support.

Then, in a following step, instead of having a predefined list of syntax elements (like bold, link or mention-here) customizable only via markdownStyle, it will be possible to use CSS styles (like color, backgroundColor, fontSize, fontWeight, fontStyle etc.), but this is a separate feature.

tomekzaw avatar Jul 18 '24 08:07 tomekzaw

Hello @tomekzaw,

I've been testing out the branch for PR #439 on iOS and the custom parsing is working great. Having an of issue with the Android dev build and I see you were working on it today. Do you have an idea on the timeline for when this PR might be merged?

Appreciate all that you do here.

szt217 avatar Sep 13 '24 20:09 szt217

Hi @szt217, thanks for testing out the PR! Happy to hear that custom parsing works well for you.

I've faced some issues with incorrect MarkdownTextInput height on RN 0.75 on Android with new arch enabled.

Realistically, I'd expect this PR to be merged next month. There are still some bugs to fix and it needs to be properly reviewed since there's a lot of files changed.

tomekzaw avatar Sep 14 '24 10:09 tomekzaw

Hello! Just curious how this is going?

puneetlath avatar Oct 31 '24 17:10 puneetlath

@puneetlath We're almost there. The feature is already implemented, the PR has been reviewed by @j-piasecki and we're waiting for a good time to merge it (we were still fixing some minor bugs in web implementation but now it's done).

There's a plan to use live-markdown to highlight search query so we'll be aiming to land this feature quite soon. Currently, @Kicu is reviewing the PR. Will keep you posted!

tomekzaw avatar Oct 31 '24 19:10 tomekzaw

Great!

puneetlath avatar Nov 01 '24 20:11 puneetlath

@puneetlath I'm glad to announce that today we finally merged https://github.com/Expensify/react-native-live-markdown/pull/439. This is a milestone in the development of the library.

What does it mean for us now?

  • We no longer need to bump react-native-live-markdown library each time ExpensiMark from expensify-common changes. From now on, MarkdownTextInput will use expensify-common from E/App. This means we can bump these two libraries in E/App more independently. Also, we will avoid parsing inconsistencies caused by using two different versions of expensify-common.
  • We can now parametrize parsing process with external data like app state. For instance, it will be possible to correctly implement short mentions. We can unblock https://github.com/Expensify/App/issues/38025.
  • We can re-use MarkdownTextInput component for other use-cases like highlighting some keywords in search query. We can unblock https://github.com/Expensify/App/issues/50949.
  • We will be able to debug and profile the parsing algorithm using the same tools as for debugging and profiling the main JS bundle.
  • We will be able to move the parsing algorithm from react-native-live-markdown to Expensify/App so its code is tracked the same way as the app code.

What do we need to do now?

  • Obviously, we need to bump react-native-live-markdown in Expensify/App to 0.1.188 (or later). @289Adam289 from Software Mansion will submit a PR with the bump.
  • There are some parts of the code I'd like to refactor but I wanted to keep them separate from this PR – I will submit them as follow-ups.

Thanks!

tomekzaw avatar Dec 04 '24 13:12 tomekzaw

Done in #439.

tomekzaw avatar Dec 04 '24 13:12 tomekzaw

That's awesome!! Thanks for the update 😄

puneetlath avatar Dec 10 '24 17:12 puneetlath