js: Enable no-jquery/no-constructor-attributes.

https://github.com/wikimedia/eslint-plugin-no-jquery/blob/master/docs/rules/no-constructor-attributes.md

The motivation for this rule is a subtle caveat buried in the
documentation:

https://api.jquery.com/jquery/#jQuery-html-attributes

“While the second argument is convenient, its flexibility can lead to
unintended consequences (e.g. $( "<input>", {size: "4"} ) calling the
.size() method instead of setting the size attribute).”

Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit is contained in:
Anders Kaseorg 2022-08-15 21:50:20 -07:00 committed by Tim Abbott
parent 098effe0a6
commit e8a30060ee
14 changed files with 75 additions and 61 deletions

View File

@ -73,6 +73,7 @@
"no-implied-eval": "error", "no-implied-eval": "error",
"no-inner-declarations": "off", "no-inner-declarations": "off",
"no-iterator": "error", "no-iterator": "error",
"no-jquery/no-constructor-attributes": "error",
"no-jquery/no-parse-html-literal": "error", "no-jquery/no-parse-html-literal": "error",
"no-label-var": "error", "no-label-var": "error",
"no-labels": "error", "no-labels": "error",

View File

@ -115,35 +115,38 @@ function make_switcher() {
return $self; return $self;
} }
mock_jquery((sel, attributes) => { mock_jquery((sel) => {
if (sel.stub) { if (sel.stub) {
// The component often redundantly re-wraps objects. // The component often redundantly re-wraps objects.
return sel; return sel;
} }
switch (sel) { switch (sel) {
case "<div>": { case "<div>":
if (attributes.class === "tab-switcher") { return {
addClass(className) {
if (className === "tab-switcher") {
return env.switcher; return env.switcher;
} }
assert.equal(className, "ind-tab");
return {
attr(attributes) {
const tab_id = attributes["data-tab-id"]; const tab_id = attributes["data-tab-id"];
assert.deepEqual( assert.deepEqual(
attributes, attributes,
[ [
{ {
class: "ind-tab",
"data-tab-key": "keyboard-shortcuts", "data-tab-key": "keyboard-shortcuts",
"data-tab-id": 0, "data-tab-id": 0,
tabindex: 0, tabindex: 0,
}, },
{ {
class: "ind-tab",
"data-tab-key": "message-formatting", "data-tab-key": "message-formatting",
"data-tab-id": 1, "data-tab-id": 1,
tabindex: 0, tabindex: 0,
}, },
{ {
class: "ind-tab",
"data-tab-key": "search-operators", "data-tab-key": "search-operators",
"data-tab-id": 2, "data-tab-id": 2,
tabindex: 0, tabindex: 0,
@ -151,7 +154,10 @@ mock_jquery((sel, attributes) => {
][tab_id], ][tab_id],
); );
return make_tab(tab_id); return make_tab(tab_id);
} },
};
},
};
/* istanbul ignore next */ /* istanbul ignore next */
default: default:
throw new Error("unknown selector: " + sel); throw new Error("unknown selector: " + sel);

View File

@ -22,6 +22,9 @@ mock_jquery((arg) => {
} }
return { return {
addClass() {
return this;
},
replace: (regex, string) => { replace: (regex, string) => {
arg = arg.replace(regex, string); arg = arg.replace(regex, string);
}, },

View File

@ -102,9 +102,9 @@ export async function display_stacktrace(error: string, stack: string): Promise<
}), }),
); );
const $alert = $("<div>", {class: "stacktrace"}).html( const $alert = $("<div>")
render_blueslip_stacktrace({error, stackframes}), .addClass("stacktrace")
); .html(render_blueslip_stacktrace({error, stackframes}));
$(".alert-box").append($alert); $(".alert-box").append($alert);
$alert.addClass("show"); $alert.addClass("show");
} }

View File

@ -32,7 +32,7 @@ export function toggle(opts: {
child_wants_focus?: boolean; child_wants_focus?: boolean;
selected?: number; selected?: number;
}): Toggle { }): Toggle {
const $component = $("<div>", {class: "tab-switcher"}); const $component = $("<div>").addClass("tab-switcher");
if (opts.html_class) { if (opts.html_class) {
// add a check inside passed arguments in case some extra // add a check inside passed arguments in case some extra
// classes need to be added for correct alignment or other purposes // classes need to be added for correct alignment or other purposes
@ -41,12 +41,9 @@ export function toggle(opts: {
for (const [i, value] of opts.values.entries()) { for (const [i, value] of opts.values.entries()) {
// create a tab with a tab-id so they don't have to be referenced // create a tab with a tab-id so they don't have to be referenced
// by text value which can be inconsistent. // by text value which can be inconsistent.
const $tab = $("<div>", { const $tab = $("<div>")
class: "ind-tab", .addClass("ind-tab")
"data-tab-key": value.key, .attr({"data-tab-key": value.key, "data-tab-id": i, tabindex: 0});
"data-tab-id": i,
tabindex: 0,
});
/* istanbul ignore if */ /* istanbul ignore if */
if (value.label_html !== undefined) { if (value.label_html !== undefined) {

View File

@ -415,7 +415,7 @@ export class MultiSelectDropdownListWidget extends DropdownListWidget {
add_check_mark($element) { add_check_mark($element) {
const value = $element.attr("data-value"); const value = $element.attr("data-value");
const $link_elem = $element.find("a").expectOne(); const $link_elem = $element.find("a").expectOne();
$link_elem.prepend($("<i>", {class: "fa fa-check"})); $link_elem.prepend($("<i>").addClass(["fa", "fa-check"]));
$element.addClass("checked"); $element.addClass("checked");
this.data_selected.push(value); this.data_selected.push(value);
} }

View File

@ -11,7 +11,7 @@ function is_numeric_key(key) {
} }
export function show_flatpickr(element, callback, default_timestamp, options = {}) { export function show_flatpickr(element, callback, default_timestamp, options = {}) {
const $flatpickr_input = $("<input>", {id: "#timestamp_flatpickr"}); const $flatpickr_input = $("<input>").attr("id", "#timestamp_flatpickr");
const instance = $flatpickr_input.flatpickr({ const instance = $flatpickr_input.flatpickr({
mode: "single", mode: "single",

View File

@ -183,10 +183,10 @@ export function render_lightbox_list_images(preview_source) {
const src = img.getAttribute("src"); const src = img.getAttribute("src");
const className = preview_source === src ? "image selected" : "image"; const className = preview_source === src ? "image selected" : "image";
const $node = $("<div>", { const $node = $("<div>")
class: className, .addClass(className)
"data-src": src, .attr("data-src", src)
}).css({backgroundImage: "url(" + src + ")"}); .css({backgroundImage: "url(" + src + ")"});
$image_list.append($node); $image_list.append($node);

View File

@ -202,7 +202,7 @@ export function create($container, list, opts) {
if ($list_item.length) { if ($list_item.length) {
const $link_elem = $list_item.find("a").expectOne(); const $link_elem = $list_item.find("a").expectOne();
$list_item.addClass("checked"); $list_item.addClass("checked");
$link_elem.prepend($("<i>", {class: "fa fa-check"})); $link_elem.prepend($("<i>").addClass(["fa", "fa-check"]));
} }
} }
} }

View File

@ -21,21 +21,23 @@ export function make_indicator(
// Create some additional containers to facilitate absolutely // Create some additional containers to facilitate absolutely
// positioned spinners. // positioned spinners.
const container_id = $container.attr("id")!; const container_id = $container.attr("id")!;
let $inner_container = $("<div>", {id: `${container_id}_box_container`}); let $inner_container = $("<div>").attr("id", `${container_id}_box_container`);
$container.append($inner_container); $container.append($inner_container);
$container = $inner_container; $container = $inner_container;
$inner_container = $("<div>", {id: `${container_id}_box`}); $inner_container = $("<div>").attr("id", `${container_id}_box`);
$container.append($inner_container); $container.append($inner_container);
$container = $inner_container; $container = $inner_container;
} }
const $spinner_elem = $("<div>", {class: "loading_indicator_spinner", ["aria-hidden"]: "true"}); const $spinner_elem = $("<div>")
.addClass("loading_indicator_spinner")
.attr("aria-hidden", "true");
$spinner_elem.html(render_loader({container_id: $outer_container.attr("id")})); $spinner_elem.html(render_loader({container_id: $outer_container.attr("id")}));
$container.append($spinner_elem); $container.append($spinner_elem);
let text_width = 0; let text_width = 0;
if (text !== undefined) { if (text !== undefined) {
const $text_elem = $("<span>", {class: "loading_indicator_text"}); const $text_elem = $("<span>").addClass("loading_indicator_text");
$text_elem.text(text); $text_elem.text(text);
$container.append($text_elem); $container.append($text_elem);
// See note, below // See note, below

View File

@ -156,8 +156,13 @@ function hide_catalog_show_integration() {
link = name; link = name;
} }
} }
const $category_el = $("<a>", {href: "/integrations/" + link}).append( const $category_el = $("<a>")
$("<h3>", {class: "integration-category", ["data-category"]: link}).text(category), .attr("href", "/integrations/" + link)
.append(
$("<h3>")
.addClass("integration-category")
.attr("data-category", link)
.text(category),
); );
$("#integration-instructions-group .categories").append($category_el); $("#integration-instructions-group .categories").append($category_el);
} }

View File

@ -95,7 +95,7 @@ export function update_view_on_deactivate(user_id) {
$row.find("i.deactivated-user-icon").show(); $row.find("i.deactivated-user-icon").show();
$button.addClass("btn-warning reactivate"); $button.addClass("btn-warning reactivate");
$button.removeClass("deactivate btn-danger"); $button.removeClass("deactivate btn-danger");
$button.empty().append($("<i>", {class: "fa fa-user-plus", ["aria-hidden"]: "true"})); $button.empty().append($("<i>").addClass(["fa", "fa-user-plus"]).attr("aria-hidden", "true"));
$button.attr("title", "Reactivate"); $button.attr("title", "Reactivate");
$row.addClass("deactivated_user"); $row.addClass("deactivated_user");
@ -115,7 +115,7 @@ function update_view_on_reactivate($row) {
$button.addClass("btn-danger deactivate"); $button.addClass("btn-danger deactivate");
$button.removeClass("btn-warning reactivate"); $button.removeClass("btn-warning reactivate");
$button.attr("title", "Deactivate"); $button.attr("title", "Deactivate");
$button.empty().append($("<i>", {class: "fa fa-user-times", ["aria-hidden"]: "true"})); $button.empty().append($("<i>").addClass(["fa", "fa-user-times"]).attr("aria-hidden", "true"));
$row.removeClass("deactivated_user"); $row.removeClass("deactivated_user");
if ($user_role) { if ($user_role) {

View File

@ -24,7 +24,7 @@ export function initialize_disable_btn_hint_popover(
$disabled_btn.css("pointer-events", "none"); $disabled_btn.css("pointer-events", "none");
$popover_btn.popover({ $popover_btn.popover({
placement: "bottom", placement: "bottom",
content: $("<div>", {class: "sub_disable_btn_hint"}).text(hint_text).prop("outerHTML"), content: $("<div>").addClass("sub_disable_btn_hint").text(hint_text).prop("outerHTML"),
trigger: "manual", trigger: "manual",
html: true, html: true,
animation: false, animation: false,

View File

@ -65,10 +65,10 @@ export function success(response_html: string, $status_box: JQuery, remove_after
} }
export function generic_embed_error(error_html: string): void { export function generic_embed_error(error_html: string): void {
const $alert = $("<div>", {class: "alert home-error-bar show"}); const $alert = $("<div>").addClass(["alert", "home-error-bar", "show"]);
const $exit = $("<div>", {class: "exit"}); const $exit = $("<div>").addClass("exit");
$(".alert-box").append($alert.append($exit, $("<div>", {class: "content"}).html(error_html))); $(".alert-box").append($alert.append($exit, $("<div>").addClass("content").html(error_html)));
} }
export function generic_row_button_error(xhr: JQuery.jqXHR, $btn: JQuery): void { export function generic_row_button_error(xhr: JQuery.jqXHR, $btn: JQuery): void {