Skip to content

Commit adeb0e7

Browse files
MartinCupelaMartinCupela
authored
fix: ensure all message links are properly wrapped when sharing same root domain (#2754)
1 parent 22e0702 commit adeb0e7

File tree

3 files changed

+91
-53
lines changed

3 files changed

+91
-53
lines changed

‎src/components/Message/renderText/__tests__/__snapshots__/renderText.test.js.snap‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,3 +951,39 @@ exports[`renderText renders standard markdown text 1`] = `
951951
</p>
952952
</div>
953953
`;
954+
955+
exports[`renderText wraps every link entirely even though the link roots are the same 1`] = `
956+
<div>
957+
<p>
958+
<a
959+
class="str-chat__message-url-link"
960+
href="http://copilot.com"
961+
rel="nofollow noreferrer noopener"
962+
target="_blank"
963+
>
964+
copilot.com
965+
</a>
966+
967+
968+
<a
969+
class="str-chat__message-url-link"
970+
href="http://copilot.com/guide"
971+
rel="nofollow noreferrer noopener"
972+
target="_blank"
973+
>
974+
copilot.com/guide
975+
</a>
976+
977+
978+
<a
979+
class="str-chat__message-url-link"
980+
href="http://copilot.com/about"
981+
rel="nofollow noreferrer noopener"
982+
target="_blank"
983+
>
984+
copilot.com/about
985+
</a>
986+
?
987+
</p>
988+
</div>
989+
`;

‎src/components/Message/renderText/__tests__/renderText.test.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import { defaultAllowedTagNames, renderText } from '../renderText';
88
const strikeThroughText = '~~xxx~~';
99

1010
describe(`renderText`, () => {
11+
it('wraps every link entirely even though the link roots are the same', () => {
12+
const Markdown = renderText('copilot.com\ncopilot.com/guide\ncopilot.com/about?');
13+
const { container } = render(Markdown);
14+
expect(container).toMatchSnapshot();
15+
});
16+
1117
it('handles the special case where user name matches to an e-mail pattern - 1', () => {
1218
const Markdown = renderText(
1319
'Hello @[email protected], is [email protected] your @primary e-mail?',

‎src/components/Message/renderText/renderText.tsx‎

Lines changed: 49 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import React from 'react';
22
import ReactMarkdown, { defaultUrlTransform } from 'react-markdown';
33
import { find } from 'linkifyjs';
4-
import uniqBy from 'lodash.uniqby';
54
import remarkGfm from 'remark-gfm';
65
import type { ComponentType } from 'react';
76
import type { Options } from 'react-markdown/lib';
87
import type { UserResponse } from 'stream-chat';
9-
import type { PluggableList } from 'unified'; // A subdependency of react-markdown. The type is not declared or re-exported from anywhere else
8+
import type { PluggableList } from 'unified'; // A sub-dependency of react-markdown. The type is not declared or re-exported from anywhere else
109

1110
import { Anchor, Emoji, Mention } from './componentRenderers';
12-
import { detectHttp, escapeRegExp, matchMarkdownLinks, messageCodeBlocks } from './regex';
11+
import { detectHttp, matchMarkdownLinks, messageCodeBlocks } from './regex';
1312
import { emojiMarkdownPlugin, mentionsMarkdownPlugin } from './rehypePlugins';
1413
import { htmlToTextPlugin, keepLineBreaksPlugin } from './remarkPlugins';
1514
import { ErrorBoundary } from '../../UtilityComponents';
@@ -108,60 +107,57 @@ export const renderText = (
108107
const markdownLinks = matchMarkdownLinks(newText);
109108
const codeBlocks = messageCodeBlocks(newText);
110109

111-
// extract all valid links/emails within text and replace it with proper markup
112-
uniqBy([...find(newText, 'email'), ...find(newText, 'url')], 'value').forEach(
113-
({ href, type, value }) => {
114-
const linkIsInBlock = codeBlocks.some((block) => block?.includes(value));
115-
116-
// check if message is already markdown
117-
const noParsingNeeded =
118-
markdownLinks &&
119-
markdownLinks.filter((text) => {
120-
const strippedHref = href?.replace(detectHttp, '');
121-
const strippedText = text?.replace(detectHttp, '');
122-
123-
if (!strippedHref || !strippedText) return false;
124-
125-
return (
126-
strippedHref.includes(strippedText) || strippedText.includes(strippedHref)
127-
);
128-
});
129-
130-
if (noParsingNeeded.length > 0 || linkIsInBlock) return;
131-
132-
try {
133-
// special case for mentions:
134-
// it could happen that a user's name matches with an e-mail format pattern.
135-
// in that case, we check whether the found e-mail is actually a mention
136-
// by naively checking for an existence of @ sign in front of it.
137-
if (type === 'email' && mentionedUsers) {
138-
const emailMatchesWithName = mentionedUsers.some((u) => u.name === value);
139-
if (emailMatchesWithName) {
140-
newText = newText.replace(
141-
new RegExp(escapeRegExp(value), 'g'),
142-
(match, position) => {
143-
const isMention = newText.charAt(position - 1) === '@';
144-
// in case of mention, we leave the match in its original form,
145-
// and we let `mentionsMarkdownPlugin` to do its job
146-
return isMention ? match : `[${match}](${encodeDecode(href)})`;
147-
},
148-
);
149-
150-
return;
151-
}
110+
// Extract all valid links/emails within text and replace it with proper markup
111+
// Revert the link order to avoid getting out of sync of the original start and end positions of links
112+
// - due to the addition of new characters when creating Markdown links
113+
const links = [...find(newText, 'email'), ...find(newText, 'url')];
114+
for (let i = links.length - 1; i >= 0; i--) {
115+
const { end, href, start, type, value } = links[i];
116+
const linkIsInBlock = codeBlocks.some((block) => block?.includes(value));
117+
118+
// check if message is already markdown
119+
const noParsingNeeded =
120+
markdownLinks &&
121+
markdownLinks.filter((text) => {
122+
const strippedHref = href?.replace(detectHttp, '');
123+
const strippedText = text?.replace(detectHttp, '');
124+
125+
if (!strippedHref || !strippedText) return false;
126+
127+
return strippedHref.includes(strippedText) || strippedText.includes(strippedHref);
128+
});
129+
130+
if (noParsingNeeded.length > 0 || linkIsInBlock) return;
131+
132+
try {
133+
// special case for mentions:
134+
// it could happen that a user's name matches with an e-mail format pattern.
135+
// in that case, we check whether the found e-mail is actually a mention
136+
// by naively checking for an existence of @ sign in front of it.
137+
if (type === 'email' && mentionedUsers) {
138+
const emailMatchesWithName = mentionedUsers.find((u) => u.name === value);
139+
if (emailMatchesWithName) {
140+
// FIXME: breaks if the mention symbol is not '@'
141+
const isMention = newText.charAt(start - 1) === '@';
142+
// in case of mention, we leave the match in its original form,
143+
// and we let `mentionsMarkdownPlugin` to do its job
144+
newText =
145+
newText.slice(0, start) +
146+
(isMention ? value : `[${value}](${encodeDecode(href)})`) +
147+
newText.slice(end);
152148
}
153-
149+
} else {
154150
const displayLink = type === 'email' ? value : formatUrlForDisplay(href);
155151

156-
newText = newText.replace(
157-
new RegExp(escapeRegExp(value), 'g'),
158-
`[${displayLink}](${encodeDecode(href)})`,
159-
);
160-
} catch (e) {
161-
void e;
152+
newText =
153+
newText.slice(0, start) +
154+
`[${displayLink}](${encodeDecode(href)})` +
155+
newText.slice(end);
162156
}
163-
},
164-
);
157+
} catch (e) {
158+
void e;
159+
}
160+
}
165161

166162
const remarkPlugins: PluggableList = [
167163
htmlToTextPlugin,

0 commit comments

Comments
 (0)