From e8a30060ee2405ce61ad5e5b15ca89bc95d4d4e0 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Mon, 15 Aug 2022 21:50:20 -0700 Subject: [PATCH] js: Enable no-jquery/no-constructor-attributes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. $( "", {size: "4"} ) calling the .size() method instead of setting the size attribute).” Signed-off-by: Anders Kaseorg --- .eslintrc.json | 1 + frontend_tests/node_tests/components.js | 68 +++++++++++++----------- frontend_tests/node_tests/list_widget.js | 3 ++ static/js/blueslip_stacktrace.ts | 6 +-- static/js/components.ts | 11 ++-- static/js/dropdown_list_widget.js | 2 +- static/js/flatpickr.js | 2 +- static/js/lightbox.js | 8 +-- static/js/list_widget.js | 2 +- static/js/loading.ts | 10 ++-- static/js/portico/integrations.js | 11 ++-- static/js/settings_users.js | 4 +- static/js/stream_ui_updates.js | 2 +- static/js/ui_report.ts | 6 +-- 14 files changed, 75 insertions(+), 61 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 46d829cbfd..d97a8cc8d0 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -73,6 +73,7 @@ "no-implied-eval": "error", "no-inner-declarations": "off", "no-iterator": "error", + "no-jquery/no-constructor-attributes": "error", "no-jquery/no-parse-html-literal": "error", "no-label-var": "error", "no-labels": "error", diff --git a/frontend_tests/node_tests/components.js b/frontend_tests/node_tests/components.js index 9bafa27a4c..dd36cbbd2c 100644 --- a/frontend_tests/node_tests/components.js +++ b/frontend_tests/node_tests/components.js @@ -115,43 +115,49 @@ function make_switcher() { return $self; } -mock_jquery((sel, attributes) => { +mock_jquery((sel) => { if (sel.stub) { // The component often redundantly re-wraps objects. return sel; } switch (sel) { - case "
": { - if (attributes.class === "tab-switcher") { - return env.switcher; - } - const tab_id = attributes["data-tab-id"]; - assert.deepEqual( - attributes, - [ - { - class: "ind-tab", - "data-tab-key": "keyboard-shortcuts", - "data-tab-id": 0, - tabindex: 0, - }, - { - class: "ind-tab", - "data-tab-key": "message-formatting", - "data-tab-id": 1, - tabindex: 0, - }, - { - class: "ind-tab", - "data-tab-key": "search-operators", - "data-tab-id": 2, - tabindex: 0, - }, - ][tab_id], - ); - return make_tab(tab_id); - } + case "
": + return { + addClass(className) { + if (className === "tab-switcher") { + return env.switcher; + } + + assert.equal(className, "ind-tab"); + return { + attr(attributes) { + const tab_id = attributes["data-tab-id"]; + assert.deepEqual( + attributes, + [ + { + "data-tab-key": "keyboard-shortcuts", + "data-tab-id": 0, + tabindex: 0, + }, + { + "data-tab-key": "message-formatting", + "data-tab-id": 1, + tabindex: 0, + }, + { + "data-tab-key": "search-operators", + "data-tab-id": 2, + tabindex: 0, + }, + ][tab_id], + ); + return make_tab(tab_id); + }, + }; + }, + }; /* istanbul ignore next */ default: throw new Error("unknown selector: " + sel); diff --git a/frontend_tests/node_tests/list_widget.js b/frontend_tests/node_tests/list_widget.js index f837bbc9e9..7e3f5d6fcd 100644 --- a/frontend_tests/node_tests/list_widget.js +++ b/frontend_tests/node_tests/list_widget.js @@ -22,6 +22,9 @@ mock_jquery((arg) => { } return { + addClass() { + return this; + }, replace: (regex, string) => { arg = arg.replace(regex, string); }, diff --git a/static/js/blueslip_stacktrace.ts b/static/js/blueslip_stacktrace.ts index f4a721c503..84553b5ff4 100644 --- a/static/js/blueslip_stacktrace.ts +++ b/static/js/blueslip_stacktrace.ts @@ -102,9 +102,9 @@ export async function display_stacktrace(error: string, stack: string): Promise< }), ); - const $alert = $("
", {class: "stacktrace"}).html( - render_blueslip_stacktrace({error, stackframes}), - ); + const $alert = $("
") + .addClass("stacktrace") + .html(render_blueslip_stacktrace({error, stackframes})); $(".alert-box").append($alert); $alert.addClass("show"); } diff --git a/static/js/components.ts b/static/js/components.ts index 796a9bf551..2badfe522e 100644 --- a/static/js/components.ts +++ b/static/js/components.ts @@ -32,7 +32,7 @@ export function toggle(opts: { child_wants_focus?: boolean; selected?: number; }): Toggle { - const $component = $("
", {class: "tab-switcher"}); + const $component = $("
").addClass("tab-switcher"); if (opts.html_class) { // add a check inside passed arguments in case some extra // 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()) { // create a tab with a tab-id so they don't have to be referenced // by text value which can be inconsistent. - const $tab = $("
", { - class: "ind-tab", - "data-tab-key": value.key, - "data-tab-id": i, - tabindex: 0, - }); + const $tab = $("
") + .addClass("ind-tab") + .attr({"data-tab-key": value.key, "data-tab-id": i, tabindex: 0}); /* istanbul ignore if */ if (value.label_html !== undefined) { diff --git a/static/js/dropdown_list_widget.js b/static/js/dropdown_list_widget.js index 362d51e023..de87e0a20b 100644 --- a/static/js/dropdown_list_widget.js +++ b/static/js/dropdown_list_widget.js @@ -415,7 +415,7 @@ export class MultiSelectDropdownListWidget extends DropdownListWidget { add_check_mark($element) { const value = $element.attr("data-value"); const $link_elem = $element.find("a").expectOne(); - $link_elem.prepend($("", {class: "fa fa-check"})); + $link_elem.prepend($("").addClass(["fa", "fa-check"])); $element.addClass("checked"); this.data_selected.push(value); } diff --git a/static/js/flatpickr.js b/static/js/flatpickr.js index 126504066a..8dee13bbf5 100644 --- a/static/js/flatpickr.js +++ b/static/js/flatpickr.js @@ -11,7 +11,7 @@ function is_numeric_key(key) { } export function show_flatpickr(element, callback, default_timestamp, options = {}) { - const $flatpickr_input = $("", {id: "#timestamp_flatpickr"}); + const $flatpickr_input = $("").attr("id", "#timestamp_flatpickr"); const instance = $flatpickr_input.flatpickr({ mode: "single", diff --git a/static/js/lightbox.js b/static/js/lightbox.js index ceb5ac547f..97e7d6b8c7 100644 --- a/static/js/lightbox.js +++ b/static/js/lightbox.js @@ -183,10 +183,10 @@ export function render_lightbox_list_images(preview_source) { const src = img.getAttribute("src"); const className = preview_source === src ? "image selected" : "image"; - const $node = $("
", { - class: className, - "data-src": src, - }).css({backgroundImage: "url(" + src + ")"}); + const $node = $("
") + .addClass(className) + .attr("data-src", src) + .css({backgroundImage: "url(" + src + ")"}); $image_list.append($node); diff --git a/static/js/list_widget.js b/static/js/list_widget.js index ee82e00904..b069d3a97d 100644 --- a/static/js/list_widget.js +++ b/static/js/list_widget.js @@ -202,7 +202,7 @@ export function create($container, list, opts) { if ($list_item.length) { const $link_elem = $list_item.find("a").expectOne(); $list_item.addClass("checked"); - $link_elem.prepend($("", {class: "fa fa-check"})); + $link_elem.prepend($("").addClass(["fa", "fa-check"])); } } } diff --git a/static/js/loading.ts b/static/js/loading.ts index 2409276a2a..4a3890e6ba 100644 --- a/static/js/loading.ts +++ b/static/js/loading.ts @@ -21,21 +21,23 @@ export function make_indicator( // Create some additional containers to facilitate absolutely // positioned spinners. const container_id = $container.attr("id")!; - let $inner_container = $("
", {id: `${container_id}_box_container`}); + let $inner_container = $("
").attr("id", `${container_id}_box_container`); $container.append($inner_container); $container = $inner_container; - $inner_container = $("
", {id: `${container_id}_box`}); + $inner_container = $("
").attr("id", `${container_id}_box`); $container.append($inner_container); $container = $inner_container; } - const $spinner_elem = $("
", {class: "loading_indicator_spinner", ["aria-hidden"]: "true"}); + const $spinner_elem = $("
") + .addClass("loading_indicator_spinner") + .attr("aria-hidden", "true"); $spinner_elem.html(render_loader({container_id: $outer_container.attr("id")})); $container.append($spinner_elem); let text_width = 0; if (text !== undefined) { - const $text_elem = $("", {class: "loading_indicator_text"}); + const $text_elem = $("").addClass("loading_indicator_text"); $text_elem.text(text); $container.append($text_elem); // See note, below diff --git a/static/js/portico/integrations.js b/static/js/portico/integrations.js index a6457a4849..21a888fc7d 100644 --- a/static/js/portico/integrations.js +++ b/static/js/portico/integrations.js @@ -156,9 +156,14 @@ function hide_catalog_show_integration() { link = name; } } - const $category_el = $("", {href: "/integrations/" + link}).append( - $("

", {class: "integration-category", ["data-category"]: link}).text(category), - ); + const $category_el = $("") + .attr("href", "/integrations/" + link) + .append( + $("

") + .addClass("integration-category") + .attr("data-category", link) + .text(category), + ); $("#integration-instructions-group .categories").append($category_el); } $("#integration-instructions-group").css({ diff --git a/static/js/settings_users.js b/static/js/settings_users.js index 11c46470ea..f8e73115d0 100644 --- a/static/js/settings_users.js +++ b/static/js/settings_users.js @@ -95,7 +95,7 @@ export function update_view_on_deactivate(user_id) { $row.find("i.deactivated-user-icon").show(); $button.addClass("btn-warning reactivate"); $button.removeClass("deactivate btn-danger"); - $button.empty().append($("", {class: "fa fa-user-plus", ["aria-hidden"]: "true"})); + $button.empty().append($("").addClass(["fa", "fa-user-plus"]).attr("aria-hidden", "true")); $button.attr("title", "Reactivate"); $row.addClass("deactivated_user"); @@ -115,7 +115,7 @@ function update_view_on_reactivate($row) { $button.addClass("btn-danger deactivate"); $button.removeClass("btn-warning reactivate"); $button.attr("title", "Deactivate"); - $button.empty().append($("", {class: "fa fa-user-times", ["aria-hidden"]: "true"})); + $button.empty().append($("").addClass(["fa", "fa-user-times"]).attr("aria-hidden", "true")); $row.removeClass("deactivated_user"); if ($user_role) { diff --git a/static/js/stream_ui_updates.js b/static/js/stream_ui_updates.js index dbcf4c5a31..f3f5a32f63 100644 --- a/static/js/stream_ui_updates.js +++ b/static/js/stream_ui_updates.js @@ -24,7 +24,7 @@ export function initialize_disable_btn_hint_popover( $disabled_btn.css("pointer-events", "none"); $popover_btn.popover({ placement: "bottom", - content: $("
", {class: "sub_disable_btn_hint"}).text(hint_text).prop("outerHTML"), + content: $("
").addClass("sub_disable_btn_hint").text(hint_text).prop("outerHTML"), trigger: "manual", html: true, animation: false, diff --git a/static/js/ui_report.ts b/static/js/ui_report.ts index 1893d856da..4b3126cf1c 100644 --- a/static/js/ui_report.ts +++ b/static/js/ui_report.ts @@ -65,10 +65,10 @@ export function success(response_html: string, $status_box: JQuery, remove_after } export function generic_embed_error(error_html: string): void { - const $alert = $("
", {class: "alert home-error-bar show"}); - const $exit = $("
", {class: "exit"}); + const $alert = $("
").addClass(["alert", "home-error-bar", "show"]); + const $exit = $("
").addClass("exit"); - $(".alert-box").append($alert.append($exit, $("
", {class: "content"}).html(error_html))); + $(".alert-box").append($alert.append($exit, $("
").addClass("content").html(error_html))); } export function generic_row_button_error(xhr: JQuery.jqXHR, $btn: JQuery): void {