markdown: Don't mutate the message in apply_markdown/render.

Needed for typescript, because we want to preserve
types, so instead of mutating a message object,
we can instead calculate return these values
for a message object before it's created in full.

This commit also renames apply_markdown
to render, see this comment
https://github.com/zulip/zulip/pull/28652#discussion_r1470514780
This commit is contained in:
evykassirer
2024-01-21 22:55:55 -08:00
committed by Tim Abbott
parent 47a5459637
commit fb47efc981
10 changed files with 160 additions and 71 deletions

View File

@@ -358,10 +358,7 @@ export function render_and_show_preview($preview_spinner, $preview_content_box,
// wrong, users will see a brief flicker of the locally
// echoed frontend rendering before receiving the
// authoritative backend rendering from the server).
const message_obj = {
raw_content: content,
};
markdown.apply_markdown(message_obj);
markdown.render(content);
}
channel.post({
url: "/json/messages/render",

View File

@@ -345,7 +345,10 @@ export function format_draft(draft) {
}
try {
markdown.apply_markdown(formatted);
formatted = {
...formatted,
...markdown.render(formatted.raw_content),
};
} catch (error) {
// In the unlikely event that there is syntax in the
// draft content which our Markdown processor is

View File

@@ -174,12 +174,15 @@ export function insert_local_message(message_request, local_id_float, insert_new
// Shallow clone of message request object that is turned into something suitable
// for zulip.js:add_message
// Keep this in sync with changes to compose.create_message_object
const message = {...message_request};
let message = {...message_request};
message.raw_content = message.content;
// NOTE: This will parse synchronously. We're not using the async pipeline
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
message.content_type = "text/html";
message.sender_email = people.my_current_email();
@@ -315,7 +318,10 @@ export function edit_locally(message, request) {
// all flags, so we need to restore those flags that are
// properties of how the user has interacted with the
// message, and not its rendering.
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
if (request.starred !== undefined) {
message.starred = request.starred;
}
@@ -336,6 +342,7 @@ export function edit_locally(message, request) {
}
stream_list.update_streams_sidebar();
pm_list.update_private_messages();
return message;
}
export function reify_message_id(local_id, server_id) {

View File

@@ -246,8 +246,10 @@ Coffee`,
export function set_up_toggler() {
for (const row of markdown_help_rows) {
if (row.markdown && !row.output_html) {
const message = {raw_content: row.markdown};
markdown.apply_markdown(message);
const message = {
raw_content: row.markdown,
...markdown.render(row.markdown),
};
row.output_html = util.clean_user_content_links(message.content);
}
}

View File

@@ -675,14 +675,15 @@ export function initialize(helper_config) {
web_app_helpers = helper_config;
}
export function apply_markdown(message) {
export function render(raw_content) {
// This is generally only intended to be called by the web app. Most
// other platforms should call parse().
const raw_content = message.raw_content;
const {content, flags} = parse({raw_content, helper_config: web_app_helpers});
message.content = content;
message.flags = flags;
message.is_me_message = is_status_message(raw_content);
return {
content,
flags,
is_me_message: is_status_message(raw_content),
};
}
export function add_topic_links(message) {

View File

@@ -854,7 +854,7 @@ export function end_message_edit(message_id) {
export function save_inline_topic_edit($row) {
const msg_list = message_lists.current;
let message_id = rows.id_for_recipient_row($row);
const message = message_lists.current.get(message_id);
let message = message_lists.current.get(message_id);
const old_topic = message.topic;
const new_topic = $row.find(".inline_topic_edit").val();
@@ -871,7 +871,7 @@ export function save_inline_topic_edit($row) {
if (message.locally_echoed) {
if (topic_changed) {
echo.edit_locally(message, {new_topic});
message = echo.edit_locally(message, {new_topic});
$row = message_lists.current.get_row(message_id);
}
end_inline_topic_edit($row);
@@ -946,7 +946,7 @@ export function save_message_row_edit($row) {
);
const msg_list = message_lists.current;
let message_id = rows.id($row);
const message = message_lists.current.get(message_id);
let message = message_lists.current.get(message_id);
let changed = false;
let edit_locally_echoed = false;
@@ -981,7 +981,7 @@ export function save_message_row_edit($row) {
if (message.locally_echoed) {
if (new_content !== message.raw_content) {
// `edit_locally` handles the case where `new_topic/new_stream_id` is undefined
echo.edit_locally(message, {
message = echo.edit_locally(message, {
raw_content: new_content,
});
$row = message_lists.current.get_row(message_id);
@@ -1027,7 +1027,7 @@ export function save_message_row_edit($row) {
// the message is acknowledged by the server.
message.local_edit_timestamp = Math.round(Date.now() / 1000);
echo.edit_locally(message, currently_echoing_messages.get(message_id));
message = echo.edit_locally(message, currently_echoing_messages.get(message_id));
$row = message_lists.current.get_row(message_id);
end_message_row_edit($row);
@@ -1054,14 +1054,14 @@ export function save_message_row_edit($row) {
message_id = rows.id($row);
if (edit_locally_echoed) {
const echoed_message = message_store.get(message_id);
let echoed_message = message_store.get(message_id);
const echo_data = currently_echoing_messages.get(message_id);
delete echoed_message.local_edit_timestamp;
currently_echoing_messages.delete(message_id);
// Restore the original content.
echo.edit_locally(echoed_message, {
echoed_message = echo.edit_locally(echoed_message, {
content: echo_data.orig_content,
raw_content: echo_data.orig_raw_content,
mentioned: echo_data.mentioned,

View File

@@ -235,7 +235,7 @@ test_ui("send_message", ({override, override_rewire, mock_template}) => {
override(compose_pm_pill, "get_emails", () => "alice@example.com");
const server_message_id = 127;
override(markdown, "apply_markdown", noop);
override(markdown, "render", noop);
override(markdown, "add_topic_links", noop);
override_rewire(echo, "try_deliver_locally", (message_request) => {
@@ -730,7 +730,7 @@ test_ui("on_events", ({override, override_rewire}) => {
assert.ok(make_indicator_called);
assert_visibilities();
let apply_markdown_called = false;
let render_called = false;
$("textarea#compose-textarea").val("foobarfoobar");
setup_visibilities();
setup_mock_markdown_contains_backend_only_syntax("foobarfoobar", false);
@@ -738,15 +738,14 @@ test_ui("on_events", ({override, override_rewire}) => {
current_message = "foobarfoobar";
override(markdown, "apply_markdown", (msg) => {
assert.equal(msg.raw_content, "foobarfoobar");
apply_markdown_called = true;
return msg;
override(markdown, "render", (raw_content) => {
assert.equal(raw_content, "foobarfoobar");
render_called = true;
});
handler(event);
assert.ok(apply_markdown_called);
assert.ok(render_called);
assert_visibilities();
assert.equal($("#compose .preview_content").html(), "Server: foobarfoobar");
})();

View File

@@ -31,7 +31,7 @@ set_global("setTimeout", (f, delay) => {
f();
});
mock_esm("../src/markdown", {
apply_markdown: noop,
render: noop,
});
mock_esm("../src/overlays", {
open_overlay: noop,

View File

@@ -221,12 +221,12 @@ run_test("insert_local_message streams", ({override}) => {
const local_id_float = 101.01;
let apply_markdown_called = false;
let render_called = false;
let add_topic_links_called = false;
let insert_message_called = false;
override(markdown, "apply_markdown", () => {
apply_markdown_called = true;
override(markdown, "render", () => {
render_called = true;
});
override(markdown, "add_topic_links", () => {
@@ -251,7 +251,7 @@ run_test("insert_local_message streams", ({override}) => {
};
echo.insert_local_message(message_request, local_id_float, insert_new_messages);
assert.ok(apply_markdown_called);
assert.ok(render_called);
assert.ok(add_topic_links_called);
assert.ok(insert_message_called);
});
@@ -274,7 +274,7 @@ run_test("insert_local_message direct message", ({override}) => {
people.initialize(page_params.user_id, params);
let add_topic_links_called = false;
let apply_markdown_called = false;
let render_called = false;
let insert_message_called = false;
const insert_new_messages = ([message]) => {
@@ -282,8 +282,8 @@ run_test("insert_local_message direct message", ({override}) => {
insert_message_called = true;
};
override(markdown, "apply_markdown", () => {
apply_markdown_called = true;
override(markdown, "render", () => {
render_called = true;
});
override(markdown, "add_topic_links", () => {
@@ -299,14 +299,14 @@ run_test("insert_local_message direct message", ({override}) => {
};
echo.insert_local_message(message_request, local_id_float, insert_new_messages);
assert.ok(add_topic_links_called);
assert.ok(apply_markdown_called);
assert.ok(render_called);
assert.ok(insert_message_called);
});
run_test("test reify_message_id", ({override}) => {
const local_id_float = 103.01;
override(markdown, "apply_markdown", noop);
override(markdown, "render", noop);
override(markdown, "add_topic_links", noop);
const message_request = {

View File

@@ -263,9 +263,12 @@ test("marked_shared", () => {
continue;
}
const message = {raw_content: test.input};
let message = {raw_content: test.input};
user_settings.translate_emoticons = test.translate_emoticons || false;
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
const output = message.content;
const error_message = `Failure in test: ${test.name}`;
if (test.marked_expected_output) {
@@ -281,20 +284,32 @@ test("marked_shared", () => {
test("message_flags", () => {
let message = {raw_content: "@**Leo**"};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.ok(!message.flags.includes("mentioned"));
message = {raw_content: "@**Cordelia, Lear's daughter**"};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.ok(message.flags.includes("mentioned"));
message = {raw_content: "@**all**"};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.ok(message.flags.includes("stream_wildcard_mentioned"));
assert.ok(!message.flags.includes("topic_wildcard_mentioned"));
message = {raw_content: "@**topic**"};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.ok(!message.flags.includes("stream_wildcard_mentioned"));
assert.ok(message.flags.includes("topic_wildcard_mentioned"));
});
@@ -617,8 +632,7 @@ test("marked", () => {
const input = test_case.input;
const expected = test_case.expected;
const message = {raw_content: input};
markdown.apply_markdown(message);
const message = markdown.render(input);
const output = message.content;
assert.equal(output, expected);
}
@@ -717,20 +731,29 @@ test("topic_links", () => {
test("message_flags", () => {
let input = "/me is testing this";
let message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, true);
assert.ok(!message.unread);
input = "/me is testing\nthis";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, true);
input = "testing this @**all** @**Cordelia, Lear's daughter**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, false);
assert.equal(message.flags.includes("mentioned"), true);
assert.equal(message.flags.includes("stream_wildcard_mentioned"), true);
@@ -738,7 +761,10 @@ test("message_flags", () => {
input = "test @**everyone**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, false);
assert.equal(message.flags.includes("stream_wildcard_mentioned"), true);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
@@ -746,7 +772,10 @@ test("message_flags", () => {
input = "test @**stream**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, false);
assert.equal(message.flags.includes("stream_wildcard_mentioned"), true);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
@@ -754,7 +783,10 @@ test("message_flags", () => {
input = "test @**topic**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.is_me_message, false);
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), true);
@@ -762,98 +794,140 @@ test("message_flags", () => {
input = "test @all";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @everyone";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @topic";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @any";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @alleycat.com";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @*hamletcharacters*";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), true);
input = "test @*backend*";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @**invalid_user**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @_**all**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "> test @**all**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @_**topic**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "> test @**topic**";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "test @_*hamletcharacters*";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
input = "> test @*hamletcharacters*";
message = {topic: "No links here", raw_content: input};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.flags.includes("stream_wildcard_mentioned"), false);
assert.equal(message.flags.includes("topic_wildcard_mentioned"), false);
assert.equal(message.flags.includes("mentioned"), false);
@@ -929,9 +1003,12 @@ test("parse_non_message", () => {
});
test("missing unicode emojis", ({override}) => {
const message = {raw_content: "\u{1F6B2}"};
let message = {raw_content: "\u{1F6B2}"};
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(
message.content,
'<p><span aria-label="bike" class="emoji emoji-1f6b2" role="img" title="bike">:bike:</span></p>',
@@ -941,7 +1018,10 @@ test("missing unicode emojis", ({override}) => {
override(emoji_codes.codepoint_to_name, "1f6b2", undefined);
markdown.initialize(markdown_config.get_helpers());
markdown.apply_markdown(message);
message = {
...message,
...markdown.render(message.raw_content),
};
assert.equal(message.content, "<p>\u{1F6B2}</p>");
});
@@ -952,7 +1032,7 @@ test("katex_throws_unexpected_exceptions", ({override_rewire}) => {
throw new Error("some-exception");
},
});
assert.throws(() => markdown.apply_markdown(message), {
assert.throws(() => markdown.render(message.raw_content), {
name: "Error",
message: "some-exception\nPlease report this to https://zulip.com/development-community/",
});