fix(utils): handle CRLF when parsing MDX imports

[scastiel]

Pre-flight checklist

• I have read the Contributing Guidelines on pull requests.
• If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
• [NA] If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

On Windows, when CRLF line endings are used, there is an issue when parsing Markdown. More specifically, the generated excerpt might be empty when the Markdown content starts with some import.

Test Plan

I added a unit tests that demonstrates what the issue was, and shows that it is now fixed with the change.

Test links

Deploy preview: https://deploy-preview-8606--docusaurus-2.netlify.app/

Related issues/PRs

None

[scastiel]

This is the fix. So far only CR was handled, not CRLF line endings.

[Josh-Cena]

I assume you mean "only LF was handled" 😄

[scastiel]

Yes, sorry 😉

[netlify]

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	3394bba
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63d93fe8f0edfd0008b828c1
😎 Deploy Preview	https://deploy-preview-8606--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[github-actions]

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟢 91	🟢 97	🟢 100	🟢 100	🟢 90	Report
/docs/installation	🟠 75	🟢 100	🟢 100	🟢 100	🟢 90	Report

[scastiel]

Not required for the fix, but I think it makes the code cleaner, and more consistent with the next if block.

[netlify]

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	ad4ed2d
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63d940b9918414000933e57d
😎 Deploy Preview	https://deploy-preview-8606--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[netlify]

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	a16c897
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63d94177e89bfe0008b992a4
😎 Deploy Preview	https://deploy-preview-8606--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[slorber]

isn't this test supposed to fail if I remove your change in createExcerpt ? Because it seems to pass before/after in all cases so not sure the test is testing anything 😅

[scastiel]

Actually, there is two ways to fix:

• handling \r in createExcerpt (what I did here, which is the “clean” fix)
• replacing fileLine === '' with !fileLine.trim() as .trim() removes \r characters (what I did here, which is not necessary but makes the code more consistent)

Only one of these changes is necessary. To me it’s more clear if both are made, but I can remove one of them :)

The test should fail if you remove both changes ;)

[slorber]

Ah ok, but you said one of the fix was not required for the fix 😅 but if I don't revert both of them, the test will pass in both cases. So the 2nd change is required in the end? not sure to understand why

[scastiel]

Sorry I wasn’t clear 😅 , let me try to rephrase:

The problem is that on Windows, line endings are \r\n by default (CRLF). In createExcerpt, this caused an issue if the file contained import lines, as imports are supposed to end with an empty line. But the condition fileLine === '' couldn’t be true here, as empty lines actually contained \r.

1. First way to fix this problem: changing the initial line split, by splitting on \r?\n instead of \n.

2. Another way to fix the issue is doing it later, when handling the import lines, by checking for line.trim() instead of just line, which would remove the extra \r. This fix is not needed if we apply the fix 1. already. But as we already check line.trim() later in the function for other purpose, I think it makes the check more consistent.

Only one of these fixes is necessary to handle CRLF properly in createExcerpt. To me the fix 1 is the cleanest, but it doesn’t hurt to keep the fix 2. too.

If you revert one of these fixes, the test should still pass. If you revert both, the test should fail.

[slorber]

I see thanks

Didn't know trim actually removed new lines too 😄 so we have a double solution here, that's why I was a bit confused