From 9de3d5ea72e36dd875c254886ad991f90f880faa Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 22 Oct 2024 01:35:25 +0200 Subject: [PATCH 1/3] Fix a number of typescript issues --- .eslintrc.yaml | 2 +- web_src/js/components/DiffCommitSelector.vue | 12 ++++---- web_src/js/features/repo-issue.ts | 2 +- web_src/js/utils/dom.ts | 2 +- web_src/js/vendor/jquery.are-you-sure.ts | 1 + web_src/js/webcomponents/absolute-date.ts | 13 +++++---- web_src/js/webcomponents/origin-url.test.ts | 6 ++-- web_src/js/webcomponents/origin-url.ts | 2 +- web_src/js/webcomponents/overflow-menu.ts | 30 +++++++++++++------- web_src/js/webcomponents/polyfills.ts | 5 ++-- 10 files changed, 43 insertions(+), 32 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 809817396ed61..8ff96de08da1b 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -288,7 +288,7 @@ rules: "@typescript-eslint/no-unnecessary-qualifier": [0] "@typescript-eslint/no-unnecessary-template-expression": [0] "@typescript-eslint/no-unnecessary-type-arguments": [0] - "@typescript-eslint/no-unnecessary-type-assertion": [2] + "@typescript-eslint/no-unnecessary-type-assertion": [2, {typesToIgnore: ["HTMLElement", "NodeListOf"]}] "@typescript-eslint/no-unnecessary-type-constraint": [2] "@typescript-eslint/no-unsafe-argument": [0] "@typescript-eslint/no-unsafe-assignment": [0] diff --git a/web_src/js/components/DiffCommitSelector.vue b/web_src/js/components/DiffCommitSelector.vue index b95e18796c68e..c78531cf9f328 100644 --- a/web_src/js/components/DiffCommitSelector.vue +++ b/web_src/js/components/DiffCommitSelector.vue @@ -142,11 +142,11 @@ export default { Object.assign(this.locale, results.locale); }, showAllChanges() { - window.location = `${this.issueLink}/files${this.queryParams}`; + window.location.assign(`${this.issueLink}/files${this.queryParams}`); }, /** Called when user clicks on since last review */ changesSinceLastReviewClick() { - window.location = `${this.issueLink}/files/${this.lastReviewCommitSha}..${this.commits.at(-1).id}${this.queryParams}`; + window.location.assign(`${this.issueLink}/files/${this.lastReviewCommitSha}..${this.commits.at(-1).id}${this.queryParams}`); }, /** Clicking on a single commit opens this specific commit */ commitClicked(commitId, newWindow = false) { @@ -154,7 +154,7 @@ export default { if (newWindow) { window.open(url); } else { - window.location = url; + window.location.assign(url); } }, /** @@ -176,14 +176,14 @@ export default { const lastCommitIdx = this.commits.findLastIndex((x) => x.selected); if (lastCommitIdx === this.commits.length - 1) { // user selected all commits - just show the normal diff page - window.location = `${this.issueLink}/files${this.queryParams}`; + window.location.assign(`${this.issueLink}/files${this.queryParams}`); } else { - window.location = `${this.issueLink}/files/${this.commits[lastCommitIdx].id}${this.queryParams}`; + window.location.assign(`${this.issueLink}/files/${this.commits[lastCommitIdx].id}${this.queryParams}`); } } else { const start = this.commits[this.commits.findIndex((x) => x.selected) - 1].id; const end = this.commits.findLast((x) => x.selected).id; - window.location = `${this.issueLink}/files/${start}..${end}${this.queryParams}`; + window.location.assign(`${this.issueLink}/files/${start}..${end}${this.queryParams}`); } } }, diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index e450f561c0a85..1a32c264b4208 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -97,7 +97,7 @@ function excludeLabel(item) { const regStr = `labels=((?:-?[0-9]+%2c)*)(${id})((?:%2c-?[0-9]+)*)&`; const newStr = 'labels=$1-$2$3&'; - window.location = href.replace(new RegExp(regStr), newStr); + window.location.assign(href.replace(new RegExp(regStr), newStr)); } export function initRepoIssueSidebarList() { diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 7dd63ecbbf01a..1e85286f3850e 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -92,7 +92,7 @@ export function onDomReady(cb: () => Promisable) { // checks whether an element is owned by the current document, and whether it is a document fragment or element node // if it is, it means it is a "normal" element managed by us, which can be modified safely. -export function isDocumentFragmentOrElementNode(el: Element) { +export function isDocumentFragmentOrElementNode(el: Element | Node) { try { return el.ownerDocument === document && el.nodeType === Node.ELEMENT_NODE || el.nodeType === Node.DOCUMENT_FRAGMENT_NODE; } catch { diff --git a/web_src/js/vendor/jquery.are-you-sure.ts b/web_src/js/vendor/jquery.are-you-sure.ts index e7bb203c5a92f..858f9871b8b4d 100644 --- a/web_src/js/vendor/jquery.are-you-sure.ts +++ b/web_src/js/vendor/jquery.are-you-sure.ts @@ -1,3 +1,4 @@ +// @ts-nocheck // Fork of the upstream module. The only changes are: // * use export to make it work with ES6 modules. // * the addition of `const` to make it strict mode compatible. diff --git a/web_src/js/webcomponents/absolute-date.ts b/web_src/js/webcomponents/absolute-date.ts index d2be455302450..ba1c72c0767f4 100644 --- a/web_src/js/webcomponents/absolute-date.ts +++ b/web_src/js/webcomponents/absolute-date.ts @@ -1,17 +1,18 @@ import {Temporal} from 'temporal-polyfill'; -export function toAbsoluteLocaleDate(dateStr, lang, opts) { +export function toAbsoluteLocaleDate(dateStr: string, lang: string, opts: Intl.DateTimeFormatOptions) { return Temporal.PlainDate.from(dateStr).toLocaleString(lang ?? [], opts); } window.customElements.define('absolute-date', class extends HTMLElement { static observedAttributes = ['date', 'year', 'month', 'weekday', 'day']; + initialized: boolean; update = () => { - const year = this.getAttribute('year') ?? ''; - const month = this.getAttribute('month') ?? ''; - const weekday = this.getAttribute('weekday') ?? ''; - const day = this.getAttribute('day') ?? ''; + const year = (this.getAttribute('year') ?? '') as Intl.DateTimeFormatOptions['year']; + const month = (this.getAttribute('month') ?? '') as Intl.DateTimeFormatOptions['month']; + const weekday = (this.getAttribute('weekday') ?? '') as Intl.DateTimeFormatOptions['weekday']; + const day = (this.getAttribute('day') ?? '') as Intl.DateTimeFormatOptions['day']; const lang = this.closest('[lang]')?.getAttribute('lang') || this.ownerDocument.documentElement.getAttribute('lang') || ''; @@ -27,7 +28,7 @@ window.customElements.define('absolute-date', class extends HTMLElement { }); }; - attributeChangedCallback(_name, oldValue, newValue) { + attributeChangedCallback(_name: string, oldValue: any, newValue: any) { if (!this.initialized || oldValue === newValue) return; this.update(); } diff --git a/web_src/js/webcomponents/origin-url.test.ts b/web_src/js/webcomponents/origin-url.test.ts index 4082e53aea921..19cc467d7dfc7 100644 --- a/web_src/js/webcomponents/origin-url.test.ts +++ b/web_src/js/webcomponents/origin-url.test.ts @@ -1,9 +1,9 @@ import {toOriginUrl} from './origin-url.ts'; test('toOriginUrl', () => { - const oldLocation = window.location; + const oldLocation = String(window.location); for (const origin of ['https://example.com', 'https://example.com:3000']) { - window.location = new URL(`${origin}/`); + window.location.assign(`${origin}/`); expect(toOriginUrl('/')).toEqual(`${origin}/`); expect(toOriginUrl('/org/repo.git')).toEqual(`${origin}/org/repo.git`); expect(toOriginUrl('https://another.com')).toEqual(`${origin}/`); @@ -13,5 +13,5 @@ test('toOriginUrl', () => { expect(toOriginUrl('https://another.com:4000/')).toEqual(`${origin}/`); expect(toOriginUrl('https://another.com:4000/org/repo.git')).toEqual(`${origin}/org/repo.git`); } - window.location = oldLocation; + window.location.assign(oldLocation); }); diff --git a/web_src/js/webcomponents/origin-url.ts b/web_src/js/webcomponents/origin-url.ts index 09aa77f2c0f14..d407fe0dff7d1 100644 --- a/web_src/js/webcomponents/origin-url.ts +++ b/web_src/js/webcomponents/origin-url.ts @@ -1,7 +1,7 @@ // Convert an absolute or relative URL to an absolute URL with the current origin. It only // processes absolute HTTP/HTTPS URLs or relative URLs like '/xxx' or '//host/xxx'. // NOTE: Keep this function in sync with clone_script.tmpl -export function toOriginUrl(urlStr) { +export function toOriginUrl(urlStr: string) { try { if (urlStr.startsWith('http://') || urlStr.startsWith('https://') || urlStr.startsWith('/')) { const {origin, protocol, hostname, port} = window.location; diff --git a/web_src/js/webcomponents/overflow-menu.ts b/web_src/js/webcomponents/overflow-menu.ts index 2efeb9222b013..777d7dc65dd39 100644 --- a/web_src/js/webcomponents/overflow-menu.ts +++ b/web_src/js/webcomponents/overflow-menu.ts @@ -4,6 +4,14 @@ import {isDocumentFragmentOrElementNode} from '../utils/dom.ts'; import octiconKebabHorizontal from '../../../public/assets/img/svg/octicon-kebab-horizontal.svg'; window.customElements.define('overflow-menu', class extends HTMLElement { + tippyContent: HTMLDivElement; + tippyItems: Array; + button: HTMLButtonElement; + menuItemsEl: HTMLElement; + resizeObserver: ResizeObserver; + mutationObserver: MutationObserver; + lastWidth: number; + updateItems = throttle(100, () => { // eslint-disable-line unicorn/consistent-function-scoping -- https://github.com/sindresorhus/eslint-plugin-unicorn/issues/2088 if (!this.tippyContent) { const div = document.createElement('div'); @@ -11,7 +19,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { div.tabIndex = -1; // for initial focus, programmatic focus only div.addEventListener('keydown', (e) => { if (e.key === 'Tab') { - const items = this.tippyContent.querySelectorAll('[role="menuitem"]'); + const items = this.tippyContent.querySelectorAll('[role="menuitem"]'); if (e.shiftKey) { if (document.activeElement === items[0]) { e.preventDefault(); @@ -32,27 +40,27 @@ window.customElements.define('overflow-menu', class extends HTMLElement { if (document.activeElement?.matches('[role="menuitem"]')) { e.preventDefault(); e.stopPropagation(); - document.activeElement.click(); + (document.activeElement as HTMLElement).click(); } } else if (e.key === 'ArrowDown') { if (document.activeElement?.matches('.tippy-target')) { e.preventDefault(); e.stopPropagation(); - document.activeElement.querySelector('[role="menuitem"]:first-of-type').focus(); + document.activeElement.querySelector('[role="menuitem"]:first-of-type').focus(); } else if (document.activeElement?.matches('[role="menuitem"]')) { e.preventDefault(); e.stopPropagation(); - document.activeElement.nextElementSibling?.focus(); + (document.activeElement.nextElementSibling as HTMLElement)?.focus(); } } else if (e.key === 'ArrowUp') { if (document.activeElement?.matches('.tippy-target')) { e.preventDefault(); e.stopPropagation(); - document.activeElement.querySelector('[role="menuitem"]:last-of-type').focus(); + document.activeElement.querySelector('[role="menuitem"]:last-of-type').focus(); } else if (document.activeElement?.matches('[role="menuitem"]')) { e.preventDefault(); e.stopPropagation(); - document.activeElement.previousElementSibling?.focus(); + (document.activeElement.previousElementSibling as HTMLElement)?.focus(); } } }); @@ -60,8 +68,8 @@ window.customElements.define('overflow-menu', class extends HTMLElement { this.tippyContent = div; } - const itemFlexSpace = this.menuItemsEl.querySelector('.item-flex-space'); - const itemOverFlowMenuButton = this.querySelector('.overflow-menu-button'); + const itemFlexSpace = this.menuItemsEl.querySelector('.item-flex-space'); + const itemOverFlowMenuButton = this.querySelector('.overflow-menu-button'); // move items in tippy back into the menu items for subsequent measurement for (const item of this.tippyItems || []) { @@ -78,7 +86,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { itemOverFlowMenuButton?.style.setProperty('display', 'none', 'important'); this.tippyItems = []; const menuRight = this.offsetLeft + this.offsetWidth; - const menuItems = this.menuItemsEl.querySelectorAll('.item, .item-flex-space'); + const menuItems = this.menuItemsEl.querySelectorAll('.item, .item-flex-space'); let afterFlexSpace = false; for (const item of menuItems) { if (item.classList.contains('item-flex-space')) { @@ -189,14 +197,14 @@ window.customElements.define('overflow-menu', class extends HTMLElement { // template rendering, wait for its addition. // The eslint rule is not sophisticated enough or aware of this problem, see // https://github.com/43081j/eslint-plugin-wc/pull/130 - const menuItemsEl = this.querySelector('.overflow-menu-items'); // eslint-disable-line wc/no-child-traversal-in-connectedcallback + const menuItemsEl = this.querySelector('.overflow-menu-items'); // eslint-disable-line wc/no-child-traversal-in-connectedcallback if (menuItemsEl) { this.menuItemsEl = menuItemsEl; this.init(); } else { this.mutationObserver = new MutationObserver((mutations) => { for (const mutation of mutations) { - for (const node of mutation.addedNodes) { + for (const node of mutation.addedNodes as NodeListOf) { if (!isDocumentFragmentOrElementNode(node)) continue; if (node.classList.contains('overflow-menu-items')) { this.menuItemsEl = node; diff --git a/web_src/js/webcomponents/polyfills.ts b/web_src/js/webcomponents/polyfills.ts index 38f50fa02f553..6d3f4c3d55ea5 100644 --- a/web_src/js/webcomponents/polyfills.ts +++ b/web_src/js/webcomponents/polyfills.ts @@ -4,10 +4,11 @@ try { new Intl.NumberFormat('en', {style: 'unit', unit: 'minute'}).format(1); } catch { const intlNumberFormat = Intl.NumberFormat; - Intl.NumberFormat = function(locales, options) { + // @ts-expect-error - polyfill is incomplete + Intl.NumberFormat = function(locales: string | string[], options: Intl.NumberFormatOptions) { if (options.style === 'unit') { return { - format(value) { + format(value: number | BigInt | string) { return ` ${value} ${options.unit}`; }, }; From b78cbd655a531af37460d1a70fa70382eac71c63 Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 22 Oct 2024 02:02:38 +0200 Subject: [PATCH 2/3] fix lint --- web_src/js/webcomponents/polyfills.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/webcomponents/polyfills.ts b/web_src/js/webcomponents/polyfills.ts index 6d3f4c3d55ea5..4a84ee9562b93 100644 --- a/web_src/js/webcomponents/polyfills.ts +++ b/web_src/js/webcomponents/polyfills.ts @@ -8,7 +8,7 @@ try { Intl.NumberFormat = function(locales: string | string[], options: Intl.NumberFormatOptions) { if (options.style === 'unit') { return { - format(value: number | BigInt | string) { + format(value: number | bigint | string) { return ` ${value} ${options.unit}`; }, }; From c5ec2fe25391a7b8d18b06b763b57afe4fad9502 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 30 Oct 2024 20:03:26 +0100 Subject: [PATCH 3/3] revert change to no-unnecessary-type-assertion as it seems to make no difference --- .eslintrc.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 8ff96de08da1b..809817396ed61 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -288,7 +288,7 @@ rules: "@typescript-eslint/no-unnecessary-qualifier": [0] "@typescript-eslint/no-unnecessary-template-expression": [0] "@typescript-eslint/no-unnecessary-type-arguments": [0] - "@typescript-eslint/no-unnecessary-type-assertion": [2, {typesToIgnore: ["HTMLElement", "NodeListOf"]}] + "@typescript-eslint/no-unnecessary-type-assertion": [2] "@typescript-eslint/no-unnecessary-type-constraint": [2] "@typescript-eslint/no-unsafe-argument": [0] "@typescript-eslint/no-unsafe-assignment": [0]