[HOLD for payment 2024-07-24] [$250] Expense - Console error shows up when opening description that contains code block

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.83-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat
3. Submit an expense with code block as description
4. Go to transaction thread
5. Wait for a while
6. Click Description
7. Might need to reopen Description a few times to see console error

Expected Result:

Console error will not show up when opening description that contains code block

Actual Result:

Console error shows up when opening description that contains code block.
Typing inside the code block creates more console errors

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6512409_1718310569210.20240614_042601.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~014b73c4727a3648d0
• Upwork Job ID: 1802541737368946970
• Last Price Increase: 2024-06-17
• Automatic offers:
• s77rt | Reviewer | 102859088
• bernhardoj | Contributor | 102859089

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[lanitochka17]

@kadiealexander FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 1

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Console error when open a money request description page that contains code block/fence.

What is the root cause of that problem?

The code block that we receive from the BE contains a \r\n as the newline. In live markdown, we will parse the markdown into tokens and tree and then parsed the tree back to the markdown to compare whether the original markdown equals to the processed markdown.
https://github.com/Expensify/react-native-live-markdown/blob/2ada06efda92c81f07044afc1467e77075206059/parser/index.ts#L242-L246

If it's different, then we get the error. As shown in the video, if we have the markdown as

```\r\nHello\r\n```

the processed input will be

```\nHello\r\n```

Why the first \r is gone? Let's first parse the markdown to tree based on the example above.

{
    tag: <>
    children: [{
        tag: "<pre>",
        children: ['Hello\r\n'],
    }],
}

Notice that the children already lost the first \r\n. When we parse the tree back to text, if it's a pre tag, the text is constructed as below:

``` + \n{children} + ```

https://github.com/Expensify/react-native-live-markdown/blob/2ada06efda92c81f07044afc1467e77075206059/parser/index.ts#L156-L160

We add the \n manually to replace the lost \r\n. However, as \r\n is not the same as \n, we got the error. Now, why the first \r\n is lost?

It's because of a markdown to HTML bug(?) in ExpensiMark.

```\r\nHello\r\n```

is parsed to

<pre>Hello\r\n</pre>

The current regex for code fence is
/(```(?:\r\n|\n))((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g

If matched, the parser will convert it to

<pre>{group}</pre>

group is the second group matched from the regex. The first group is
(```(?:\r\n|\n))
and the second group is
((?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))

Notice that the first group matches the first line break, that's why the first line break is lost.

What changes do you think we should make in order to solve the problem?

The simplest and safest way to solve this is to replace every kind of line break (CONST.REGEX.LINE_BREAK) with \n on the description page.

App/src/pages/iou/request/step/IOURequestStepDescription.tsx

Line 168 in ab7060c

defaultValue={currentDescription}

But if we want to fix the root cause, then we need to:

Update the codefence regex to include the first line break to the second group.
updated regex:
/(```)((?:\r\n|\n)(?:\s*?(?!(?:\r\n|\n)?```(?!`))[\S])+\s*?(?:\r\n|\n))(```)/g
Then, remove the adding the \n manually in live markdown parser.
https://github.com/Expensify/react-native-live-markdown/blob/2ada06efda92c81f07044afc1467e77075206059/parser/index.ts#L159

-addChildrenWithStyle(`\n${content}`, 'pre');
+addChildrenWithStyle(content, 'pre');

What alternative solutions did you explore? (Optional)

We can apply the fix in react-native-live-markdown. In parseExpensiMarkToRanges, replace all kinds of newlines to \n of the markdown input.
https://github.com/Expensify/react-native-live-markdown/blob/aa5cbefa5e248159f81393b7a2dd8adfcc807e1c/parser/index.ts#L240-L242

const cleanedMarkdown = markdown.replace(/\r\n|\r/g, '\n');

Then replace all usages of markdown in parseExpensiMarkToRanges with cleanedMarkdown

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~014b73c4727a3648d0

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[nurhesen]

Contributor details
Your Expensify account email: nurhesen@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01769ca3bf311ef116?s=1110580755107926016

Hi. I have solved the problem by converting \r\n to unicode. Just one line of code needed to add

output.mp4

[melvin-bot]

📣 @nurhesen! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[nurhesen]

Contributor details
Your Expensify account email: nurhesen@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01769ca3bf311ef116?s=1110580755107926016

Hi. I have solved the problem by converting \r\n to unicode. Just one line of code needed to add

output.mp4

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[nurhesen]

Contributor details
Your Expensify account email: nurhesen@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01769ca3bf311ef116

Hi. I have solved the problem by converting \r\n to unicode. Just one line of code needed to add

output.mp4

[melvin-bot]

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

[nurhesen]

Contributor details
Your Expensify account email: nurhesen@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01769ca3bf311ef116

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[nurhesen]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Console error shows up when opening description that contains code block

What is the root cause of that problem?

react-native-live-markdown doesn't process \r\n correctly

What changes do you think we should make in order to solve the problem?

Converting \r\n to unicode fixes the problem. Just adding one line of code needed. Here is the video proof that converting \r\n to unicode solves the problem.

output.mp4

What alternative solutions did you explore? (Optional)

Alternatively single "\n"s can be converted in the view to "\r\n" in order to keep the consistency

[s77rt]

@bernhardoj Thanks for the proposal. Your RCA makes sense. But the first newline being consumed is actually expected because the syntax for the code block must have a new line:

```Hello```

This is not a valid code block

```\r\nHello\r\n```

This is a valid code block and the content is Hello

In the returned children: ['Hello\r\n'], I would expect to see only Hello (the second newline should be consumed too but that's not our bug here).

Can we just replace all \r\n of the input with \n but at react-native-live-markdown level?

[s77rt]

@nurhesen Thanks for the proposal. Your RCA is not clear. Can you please elaborate?

[bernhardoj]

Ah, I see. I remember

```Hello```

is a valid code block. I didn't realize it's changed now. (or maybe that's always the case?)

Updated my proposal to replace the new line in react-native-live-markdown.

[BartoszGrajdek]

Hi! I'm Bartosz from SWM react-native-live-markdown team. There were some changes in the process of handling live markdown related bugs, so I'll be helping you with the proposal review to ensure high quality solutions & avoid any regressions in the react-native-live-markdown lib 🙌🏻

I'll try to review existing proposals / triage the bug later today 👀

[s77rt]

@bernhardoj Thanks for the update! That looks good to me.

🎀 👀 🎀 C+ reviewed
Link to proposal

cc @BartoszGrajdek

[bernhardoj]

The react-native-live-markdown PR is ready: Expensify/react-native-live-markdown#413

cc: @s77rt @BartoszGrajdek

[bernhardoj]

App PR is ready

cc: @s77rt

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.7-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Bump react-native-live-markdown #44958

If no regressions arise, payment will be issued on 2024-07-24. 🎊

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@s77rt] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bernhardoj]

I'll request in ND once payment is due.

[s77rt]

• The PR that introduced the bug has been identified: This existed since day one
• The offending PR has been commented on: n/a
• A discussion in #expensify-bugs has been started: Not needed (unique bug)
• Determine if we should create a regression test for this bug: No

[kadiealexander]

@bernhardoj I see you've accepted the Upwork contract, would you prefer to be paid via NewDot?

[bernhardoj]

Yes please, I have ended the UW contract and requested in ND instead.

[JmillsExpensify]

Can I get a payment summary for ND payment approval?

[kadiealexander]

Payouts due:

• Contributor: $250 @bernhardoj via NewDot
• Contributor+: $250 @s77rt via NewDot

[JmillsExpensify]

$250 approved for @bernhardoj

[JmillsExpensify]

$250 approved for @s77rt