From 466ecf191af65c453bb3e38d867e57fc211dc5c5 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Apr 2020 21:23:40 +0100 Subject: [PATCH 1/8] move urlSearchParamsToObject and global.d.ts to react-sdk Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- package.json | 1 + src/@types/global.d.ts | 31 ++++++++++++++++++++++++++ src/utils/{UrlUtils.js => UrlUtils.ts} | 6 ++++- yarn.lock | 5 +++++ 4 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 src/@types/global.d.ts rename src/utils/{UrlUtils.js => UrlUtils.ts} (89%) diff --git a/package.json b/package.json index 616f3f541e..11803d321d 100644 --- a/package.json +++ b/package.json @@ -118,6 +118,7 @@ "@babel/register": "^7.7.4", "@peculiar/webcrypto": "^1.0.22", "@types/classnames": "^2.2.10", + "@types/modernizr": "^3.5.3", "@types/react": "16.9", "babel-eslint": "^10.0.3", "babel-jest": "^24.9.0", diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts new file mode 100644 index 0000000000..963ba9d702 --- /dev/null +++ b/src/@types/global.d.ts @@ -0,0 +1,31 @@ +/* +Copyright 2020 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import * as ModernizrStatic from "modernizr"; + +declare global { + interface Window { + Modernizr: ModernizrStatic; + Olm: { + init: () => Promise; + }; + } + + // workaround for https://github.com/microsoft/TypeScript/issues/30933 + interface ObjectConstructor { + fromEntries?(xs: [string|number|symbol, any][]): object + } +} diff --git a/src/utils/UrlUtils.js b/src/utils/UrlUtils.ts similarity index 89% rename from src/utils/UrlUtils.js rename to src/utils/UrlUtils.ts index 7b207c128e..7fe5ad0c89 100644 --- a/src/utils/UrlUtils.js +++ b/src/utils/UrlUtils.ts @@ -14,7 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. */ -import url from "url"; +import * as url from "url"; + +export function urlSearchParamsToObject(params: URLSearchParams) { + return Object.fromEntries([...params.entries()]); +} /** * If a url has no path component, etc. abbreviate it to just the hostname diff --git a/yarn.lock b/yarn.lock index 538a049bff..9c57ccf649 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1257,6 +1257,11 @@ resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d" integrity sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA== +"@types/modernizr@^3.5.3": + version "3.5.3" + resolved "https://registry.yarnpkg.com/@types/modernizr/-/modernizr-3.5.3.tgz#8ef99e6252191c1d88647809109dc29884ba6d7a" + integrity sha512-jhMOZSS0UGYTS9pqvt6q3wtT3uvOSve5piTEmTMx3zzTuBLvSIMxSIBIc3d5lajVD5h4xc41AMZD2M5orN3PxA== + "@types/node@*": version "13.11.0" resolved "https://registry.yarnpkg.com/@types/node/-/node-13.11.0.tgz#390ea202539c61c8fa6ba4428b57e05bc36dc47b" From af4ef38a4110f3cd785ae826b3271f28ade11012 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Apr 2020 21:28:23 +0100 Subject: [PATCH 2/8] remove dependency on `qs` Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- package.json | 1 - src/components/views/elements/AppTile.js | 4 +- src/utils/HostingLink.js | 4 +- yarn.lock | 197 ++--------------------- 4 files changed, 16 insertions(+), 190 deletions(-) diff --git a/package.json b/package.json index 11803d321d..7b66c95d28 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,6 @@ "prop-types": "^15.5.8", "qrcode": "^1.4.4", "qrcode-react": "^0.1.16", - "qs": "^6.6.0", "react": "^16.9.0", "react-addons-css-transition-group": "15.6.2", "react-beautiful-dnd": "^4.0.1", diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 73ed605edd..8762eb449e 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -18,7 +18,6 @@ limitations under the License. */ import url from 'url'; -import qs from 'qs'; import React, {createRef} from 'react'; import PropTypes from 'prop-types'; import {MatrixClientPeg} from '../../../MatrixClientPeg'; @@ -38,6 +37,7 @@ import {IntegrationManagers} from "../../../integrations/IntegrationManagers"; import SettingsStore, {SettingLevel} from "../../../settings/SettingsStore"; import {aboveLeftOf, ContextMenu, ContextMenuButton} from "../../structures/ContextMenu"; import PersistedElement from "./PersistedElement"; +import {urlSearchParamsToObject} from "../../../utils/UrlUtils"; const ALLOWED_APP_URL_SCHEMES = ['https:', 'http:']; const ENABLE_REACT_PERF = false; @@ -234,7 +234,7 @@ export default class AppTile extends React.Component { // Append scalar_token as a query param if not already present this._scalarClient.scalarToken = token; const u = url.parse(this._addWurlParams(this.props.app.url)); - const params = qs.parse(u.query); + const params = urlSearchParamsToObject(new URLSearchParams(u.query)); if (!params.scalar_token) { params.scalar_token = encodeURIComponent(token); // u.search must be set to undefined, so that u.format() uses query parameters - https://nodejs.org/docs/latest/api/url.html#url_url_format_url_options diff --git a/src/utils/HostingLink.js b/src/utils/HostingLink.js index 580ed00de5..fce2f104bd 100644 --- a/src/utils/HostingLink.js +++ b/src/utils/HostingLink.js @@ -15,10 +15,10 @@ limitations under the License. */ import url from 'url'; -import qs from 'qs'; import SdkConfig from '../SdkConfig'; import {MatrixClientPeg} from '../MatrixClientPeg'; +import {urlSearchParamsToObject} from "./UrlUtils"; export function getHostingLink(campaign) { const hostingLink = SdkConfig.get().hosting_signup_link; @@ -29,7 +29,7 @@ export function getHostingLink(campaign) { try { const hostingUrl = url.parse(hostingLink); - const params = qs.parse(hostingUrl.query); + const params = urlSearchParamsToObject(new URLSearchParams(hostingUrl.query)); params.utm_campaign = campaign; hostingUrl.search = undefined; hostingUrl.query = params; diff --git a/yarn.lock b/yarn.lock index 9c57ccf649..8dda986b46 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1501,11 +1501,6 @@ abab@^2.0.0: resolved "https://registry.yarnpkg.com/abab/-/abab-2.0.3.tgz#623e2075e02eb2d3f2475e49f99c91846467907a" integrity sha512-tsFzPpcttalNjFBCFMqsKYQcWxxen1pgJR56by//QwvJc4/OUS3kPOOttx2tSIfjsylB0pYu7f5D3K1RCxUnUg== -abbrev@1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.1.1.tgz#f8f2c887ad10bf67f634f005b6987fed3179aac8" - integrity sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q== - acorn-globals@^4.1.0: version "4.3.4" resolved "https://registry.yarnpkg.com/acorn-globals/-/acorn-globals-4.3.4.tgz#9fa1926addc11c97308c4e66d7add0d40c3272e7" @@ -1639,19 +1634,11 @@ anymatch@~3.1.1: normalize-path "^3.0.0" picomatch "^2.0.4" -aproba@^1.0.3, aproba@^1.1.1: +aproba@^1.1.1: version "1.2.0" resolved "https://registry.yarnpkg.com/aproba/-/aproba-1.2.0.tgz#6802e6264efd18c790a1b0d517f0f2627bf2c94a" integrity sha512-Y9J6ZjXtoYh8RnXVCMOU/ttDmk1aBjunq9vO0ta5x85WDQiQfUF9sIPBITdbiiIVcBo03Hi3jMxigBtsddlXRw== -are-we-there-yet@~1.1.2: - version "1.1.5" - resolved "https://registry.yarnpkg.com/are-we-there-yet/-/are-we-there-yet-1.1.5.tgz#4b35c2944f062a8bfcda66410760350fe9ddfc21" - integrity sha512-5hYdAkZlcG8tOLujVDTgCT+uPX0VnpAH28gWsLfzpXYm7wP6mp5Q/gYyR7YQ0cKVJcXJnl3j2kpBan13PtQf6w== - dependencies: - delegates "^1.0.0" - readable-stream "^2.0.6" - argparse@^1.0.7: version "1.0.10" resolved "https://registry.yarnpkg.com/argparse/-/argparse-1.0.10.tgz#bcd6791ea5ae09725e17e5ad988134cd40b3d911" @@ -2558,11 +2545,6 @@ console-browserify@^1.1.0: resolved "https://registry.yarnpkg.com/console-browserify/-/console-browserify-1.2.0.tgz#67063cef57ceb6cf4993a2ab3a55840ae8c49336" integrity sha512-ZMkYO/LkF17QvCPqM0gxw8yUzigAOZOSWSHg91FH6orS7vcEj5dVZTidN2fQ14yBSdg97RqhSNwLUXInd52OTA== -console-control-strings@^1.0.0, console-control-strings@~1.1.0: - version "1.1.0" - resolved "https://registry.yarnpkg.com/console-control-strings/-/console-control-strings-1.1.0.tgz#3d7cf4464db6446ea644bf4b39507f9851008e8e" - integrity sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4= - constants-browserify@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/constants-browserify/-/constants-browserify-1.0.0.tgz#c20b96d8c617748aaf1c16021760cd27fcb8cb75" @@ -2808,7 +2790,7 @@ debug@^2.2.0, debug@^2.3.3: dependencies: ms "2.0.0" -debug@^3.1.0, debug@^3.2.6: +debug@^3.1.0: version "3.2.6" resolved "https://registry.yarnpkg.com/debug/-/debug-3.2.6.tgz#e83d17de16d8a7efb7717edbe5fb10135eee629b" integrity sha512-mel+jf7nrtEl5Pn1Qx46zARXKDpBbvzezse7p7LqINmdoIk8PYP5SySaxEmYv6TZ0JyEKA1hsCId6DIhgITtWQ== @@ -2884,11 +2866,6 @@ delayed-stream@~1.0.0: resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-1.0.0.tgz#df3ae199acadfb7d440aaae0b29e2272b24ec619" integrity sha1-3zrhmayt+31ECqrgsp4icrJOxhk= -delegates@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/delegates/-/delegates-1.0.0.tgz#84c6e159b81904fdca59a0ef44cd870d31250f9a" - integrity sha1-hMbhWbgZBP3KWaDvRM2HDTElD5o= - des.js@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/des.js/-/des.js-1.0.1.tgz#5382142e1bdc53f85d86d53e5f4aa7deb91e0843" @@ -2902,11 +2879,6 @@ detect-file@^1.0.0: resolved "https://registry.yarnpkg.com/detect-file/-/detect-file-1.0.0.tgz#f0d66d03672a825cb1b73bdb3fe62310c8e552b7" integrity sha1-8NZtA2cqglyxtzvbP+YjEMjlUrc= -detect-libc@^1.0.2: - version "1.0.3" - resolved "https://registry.yarnpkg.com/detect-libc/-/detect-libc-1.0.3.tgz#fa137c4bd698edf55cd5cd02ac559f91a4c4ba9b" - integrity sha1-+hN8S9aY7fVc1c0CrFWfkaTEups= - detect-newline@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/detect-newline/-/detect-newline-2.1.0.tgz#f41f1c10be4b00e87b5f13da680759f2c5bfd3e2" @@ -3921,13 +3893,6 @@ from2@^2.1.0: inherits "^2.0.1" readable-stream "^2.0.0" -fs-minipass@^1.2.5: - version "1.2.7" - resolved "https://registry.yarnpkg.com/fs-minipass/-/fs-minipass-1.2.7.tgz#ccff8570841e7fe4265693da88936c55aed7f7c7" - integrity sha512-GWSSJGFy4e9GUeCcbIkED+bgAoFyj7XF1mV8rma3QW4NIqX9Kyx79N/PF61H5udOV3aY1IaMLs6pGbH71nlCTA== - dependencies: - minipass "^2.6.0" - fs-readdir-recursive@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/fs-readdir-recursive/-/fs-readdir-recursive-1.1.0.tgz#e32fc030a2ccee44a6b5371308da54be0b397d27" @@ -3990,20 +3955,6 @@ fuse.js@^2.2.0: resolved "https://registry.yarnpkg.com/fuse.js/-/fuse.js-2.7.4.tgz#96e420fde7ef011ac49c258a621314fe576536f9" integrity sha1-luQg/efvARrEnCWKYhMU/ldlNvk= -gauge@~2.7.3: - version "2.7.4" - resolved "https://registry.yarnpkg.com/gauge/-/gauge-2.7.4.tgz#2c03405c7538c39d7eb37b317022e325fb018bf7" - integrity sha1-LANAXHU4w51+s3sxcCLjJfsBi/c= - dependencies: - aproba "^1.0.3" - console-control-strings "^1.0.0" - has-unicode "^2.0.0" - object-assign "^4.1.0" - signal-exit "^3.0.0" - string-width "^1.0.1" - strip-ansi "^3.0.1" - wide-align "^1.1.0" - gensync@^1.0.0-beta.1: version "1.0.0-beta.1" resolved "https://registry.yarnpkg.com/gensync/-/gensync-1.0.0-beta.1.tgz#58f4361ff987e5ff6e1e7a210827aa371eaac269" @@ -4201,11 +4152,6 @@ has-symbols@^1.0.0, has-symbols@^1.0.1: resolved "https://registry.yarnpkg.com/has-symbols/-/has-symbols-1.0.1.tgz#9f5214758a44196c406d9bd76cebf81ec2dd31e8" integrity sha512-PLcsoqu++dmEIZB+6totNFKq/7Do+Z0u4oT0zKOJNl3lYK6vGwwu2hjHs+68OEZbTjiUE9bgOABXbP/GvrS0Kg== -has-unicode@^2.0.0: - version "2.0.1" - resolved "https://registry.yarnpkg.com/has-unicode/-/has-unicode-2.0.1.tgz#e0e6fe6a28cf51138855e086d1691e771de2a8b9" - integrity sha1-4Ob+aijPUROIVeCG0Wkedx3iqLk= - has-value@^0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/has-value/-/has-value-0.3.1.tgz#7b1f58bada62ca827ec0a2078025654845995e1f" @@ -4393,7 +4339,7 @@ humanize-ms@^1.2.1: dependencies: ms "^2.0.0" -iconv-lite@0.4.24, iconv-lite@^0.4.24, iconv-lite@^0.4.4, iconv-lite@~0.4.13: +iconv-lite@0.4.24, iconv-lite@^0.4.24, iconv-lite@~0.4.13: version "0.4.24" resolved "https://registry.yarnpkg.com/iconv-lite/-/iconv-lite-0.4.24.tgz#2022b4b25fbddc21d2f524974a474aafe733908b" integrity sha512-v3MXnZAcvnywkTUEZomIActle7RXXeedOR31wwl7VlyoXO4Qi9arvSenNQWne1TcRwhCL1HwLI21bEqdpj8/rA== @@ -4410,13 +4356,6 @@ iferr@^0.1.5: resolved "https://registry.yarnpkg.com/iferr/-/iferr-0.1.5.tgz#c60eed69e6d8fdb6b3104a1fcbca1c192dc5b501" integrity sha1-xg7taebY/bazEEofy8ocGS3FtQE= -ignore-walk@^3.0.1: - version "3.0.3" - resolved "https://registry.yarnpkg.com/ignore-walk/-/ignore-walk-3.0.3.tgz#017e2447184bfeade7c238e4aefdd1e8f95b1e37" - integrity sha512-m7o6xuOaT1aqheYHKf8W6J5pYH85ZI9w077erOzLje3JsB1gkafkAhHHY19dqjulgIZHFm32Cp5uNZgcQqdJKw== - dependencies: - minimatch "^3.0.4" - ignore@^4.0.3, ignore@^4.0.6: version "4.0.6" resolved "https://registry.yarnpkg.com/ignore/-/ignore-4.0.6.tgz#750e3db5862087b4737ebac8207ffd1ef27b25fc" @@ -5980,21 +5919,6 @@ minimist@^1.1.1, minimist@^1.2.0, minimist@^1.2.5, "minimist@~ 1.2.0": resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.5.tgz#67d66014b66a6a8aaa0c083c5fd58df4e4e97602" integrity sha512-FM9nNUYrRBAELZQT3xeZQ7fmMOBg6nWNmJKTcgsJeaLstP/UODVpGsr5OhXhhXg6f+qtJ8uiZ+PUxkDWcgIXLw== -minipass@^2.6.0, minipass@^2.8.6, minipass@^2.9.0: - version "2.9.0" - resolved "https://registry.yarnpkg.com/minipass/-/minipass-2.9.0.tgz#e713762e7d3e32fed803115cf93e04bca9fcc9a6" - integrity sha512-wxfUjg9WebH+CUDX/CdbRlh5SmfZiy/hpkxaRI16Y9W56Pa75sWgd/rvFilSgrauD9NyFymP/+JFV3KwzIsJeg== - dependencies: - safe-buffer "^5.1.2" - yallist "^3.0.0" - -minizlib@^1.2.1: - version "1.3.3" - resolved "https://registry.yarnpkg.com/minizlib/-/minizlib-1.3.3.tgz#2290de96818a34c29551c8a8d301216bd65a861d" - integrity sha512-6ZYMOEnmVsdCeTJVE0W9ZD+pVnE8h9Hma/iOwwRDsdQoePpoX56/8B6z3P9VNwppJuBKNRuFDRNRqRWexT9G9Q== - dependencies: - minipass "^2.9.0" - mississippi@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/mississippi/-/mississippi-3.0.0.tgz#ea0a3291f97e0b5e8776b363d5f0a12d94c67022" @@ -6019,7 +5943,7 @@ mixin-deep@^1.2.0: for-in "^1.0.2" is-extendable "^1.0.1" -mkdirp@^0.5.0, mkdirp@^0.5.1, mkdirp@^0.5.3: +mkdirp@^0.5.1, mkdirp@^0.5.3: version "0.5.5" resolved "https://registry.yarnpkg.com/mkdirp/-/mkdirp-0.5.5.tgz#d91cefd62d1436ca0f41620e251288d420099def" integrity sha512-NKmAlESf6jMGym1++R0Ra7wvhV+wFW63FaSOFPwRahvea0gMUcGUhVeAg/0BC0wiv9ih5NYPB1Wn1UEI1/L+xQ== @@ -6096,15 +6020,6 @@ nearley@^2.7.10: randexp "0.4.6" semver "^5.4.1" -needle@^2.2.1: - version "2.4.1" - resolved "https://registry.yarnpkg.com/needle/-/needle-2.4.1.tgz#14af48732463d7475696f937626b1b993247a56a" - integrity sha512-x/gi6ijr4B7fwl6WYL9FwlCvRQKGlUNvnceho8wxkwXqN8jvVmmmATTmZPRRG7b/yC1eode26C2HO9jl78Du9g== - dependencies: - debug "^3.2.6" - iconv-lite "^0.4.4" - sax "^1.2.4" - neo-async@^2.5.0, neo-async@^2.6.1: version "2.6.1" resolved "https://registry.yarnpkg.com/neo-async/-/neo-async-2.6.1.tgz#ac27ada66167fa8849a6addd837f6b189ad2081c" @@ -6182,35 +6097,11 @@ node-notifier@^5.4.2: shellwords "^0.1.1" which "^1.3.0" -node-pre-gyp@*: - version "0.14.0" - resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.14.0.tgz#9a0596533b877289bcad4e143982ca3d904ddc83" - integrity sha512-+CvDC7ZttU/sSt9rFjix/P05iS43qHCOOGzcr3Ry99bXG7VX953+vFyEuph/tfqoYu8dttBkE86JSKBO2OzcxA== - dependencies: - detect-libc "^1.0.2" - mkdirp "^0.5.1" - needle "^2.2.1" - nopt "^4.0.1" - npm-packlist "^1.1.6" - npmlog "^4.0.2" - rc "^1.2.7" - rimraf "^2.6.1" - semver "^5.3.0" - tar "^4.4.2" - node-releases@^1.1.53: version "1.1.53" resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.53.tgz#2d821bfa499ed7c5dffc5e2f28c88e78a08ee3f4" integrity sha512-wp8zyQVwef2hpZ/dJH7SfSrIPD6YoJz6BDQDpGEkcA0s3LpAQoxBIYmfIq6QAhC1DhwsyCgTaTTcONwX8qzCuQ== -nopt@^4.0.1: - version "4.0.3" - resolved "https://registry.yarnpkg.com/nopt/-/nopt-4.0.3.tgz#a375cad9d02fd921278d954c2254d5aa57e15e48" - integrity sha512-CvaGwVMztSMJLOeXPrez7fyfObdZqNUK1cPAEzLHrTybIua9pMdmmPR5YwtfNftIOMv3DPUhFaxsZMNTQO20Kg== - dependencies: - abbrev "1" - osenv "^0.1.4" - normalize-package-data@^2.3.2, normalize-package-data@^2.3.4: version "2.5.0" resolved "https://registry.yarnpkg.com/normalize-package-data/-/normalize-package-data-2.5.0.tgz#e66db1838b200c1dfc233225d12cb36520e234a8" @@ -6243,27 +6134,6 @@ normalize-selector@^0.2.0: resolved "https://registry.yarnpkg.com/normalize-selector/-/normalize-selector-0.2.0.tgz#d0b145eb691189c63a78d201dc4fdb1293ef0c03" integrity sha1-0LFF62kRicY6eNIB3E/bEpPvDAM= -npm-bundled@^1.0.1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/npm-bundled/-/npm-bundled-1.1.1.tgz#1edd570865a94cdb1bc8220775e29466c9fb234b" - integrity sha512-gqkfgGePhTpAEgUsGEgcq1rqPXA+tv/aVBlgEzfXwA1yiUJF7xtEt3CtVwOjNYQOVknDk0F20w58Fnm3EtG0fA== - dependencies: - npm-normalize-package-bin "^1.0.1" - -npm-normalize-package-bin@^1.0.1: - version "1.0.1" - resolved "https://registry.yarnpkg.com/npm-normalize-package-bin/-/npm-normalize-package-bin-1.0.1.tgz#6e79a41f23fd235c0623218228da7d9c23b8f6e2" - integrity sha512-EPfafl6JL5/rU+ot6P3gRSCpPDW5VmIzX959Ob1+ySFUuuYHWHekXpwdUZcKP5C+DS4GEtdJluwBjnsNDl+fSA== - -npm-packlist@^1.1.6: - version "1.4.8" - resolved "https://registry.yarnpkg.com/npm-packlist/-/npm-packlist-1.4.8.tgz#56ee6cc135b9f98ad3d51c1c95da22bbb9b2ef3e" - integrity sha512-5+AZgwru5IevF5ZdnFglB5wNlHG1AOOuw28WhUq8/8emhBmLv6jX5by4WJCh7lW0uSYZYS6DXqIsyZVIXRZU9A== - dependencies: - ignore-walk "^3.0.1" - npm-bundled "^1.0.1" - npm-normalize-package-bin "^1.0.1" - npm-run-path@^2.0.0: version "2.0.2" resolved "https://registry.yarnpkg.com/npm-run-path/-/npm-run-path-2.0.2.tgz#35a9232dfa35d7067b4cb2ddf2357b1871536c5f" @@ -6271,16 +6141,6 @@ npm-run-path@^2.0.0: dependencies: path-key "^2.0.0" -npmlog@^4.0.2: - version "4.1.2" - resolved "https://registry.yarnpkg.com/npmlog/-/npmlog-4.1.2.tgz#08a7f2a8bf734604779a9efa4ad5cc717abb954b" - integrity sha512-2uUqazuKlTaSI/dC8AzicUck7+IrEaOnN/e0jd3Xtt1KcGpwx30v50mL7oPyr/h9bL3E4aZccVwpwP+5W9Vjkg== - dependencies: - are-we-there-yet "~1.1.2" - console-control-strings "~1.1.0" - gauge "~2.7.3" - set-blocking "~2.0.0" - nth-check@~1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/nth-check/-/nth-check-1.0.2.tgz#b2bd295c37e3dd58a3bf0700376663ba4d9cf05c" @@ -6430,11 +6290,6 @@ os-browserify@^0.3.0: resolved "https://registry.yarnpkg.com/os-browserify/-/os-browserify-0.3.0.tgz#854373c7f5c2315914fc9bfc6bd8238fdda1ec27" integrity sha1-hUNzx/XCMVkU/Jv8a9gjj92h7Cc= -os-homedir@^1.0.0: - version "1.0.2" - resolved "https://registry.yarnpkg.com/os-homedir/-/os-homedir-1.0.2.tgz#ffbc4988336e0e833de0c168c7ef152121aa7fb3" - integrity sha1-/7xJiDNuDoM94MFox+8VISGqf7M= - os-locale@^3.0.0, os-locale@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/os-locale/-/os-locale-3.1.0.tgz#a802a6ee17f24c10483ab9935719cef4ed16bf1a" @@ -6444,19 +6299,11 @@ os-locale@^3.0.0, os-locale@^3.1.0: lcid "^2.0.0" mem "^4.0.0" -os-tmpdir@^1.0.0, os-tmpdir@~1.0.2: +os-tmpdir@~1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/os-tmpdir/-/os-tmpdir-1.0.2.tgz#bbe67406c79aa85c5cfec766fe5734555dfa1274" integrity sha1-u+Z0BseaqFxc/sdm/lc0VV36EnQ= -osenv@^0.1.4: - version "0.1.5" - resolved "https://registry.yarnpkg.com/osenv/-/osenv-0.1.5.tgz#85cdfafaeb28e8677f416e287592b5f3f49ea410" - integrity sha512-0CWcCECdMVc2Rw3U5w9ZjqX6ga6ubk1xDVKxtBQPK7wis/0F2r9T6k4ydGYhecl7YUBxBVxhL5oisPsNxAPe2g== - dependencies: - os-homedir "^1.0.0" - os-tmpdir "^1.0.0" - p-defer@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/p-defer/-/p-defer-1.0.0.tgz#9f6eb182f6c9aa8cd743004a7d4f96b196b0fb0c" @@ -7036,7 +6883,7 @@ qrcode@^1.4.4: pngjs "^3.3.0" yargs "^13.2.4" -qs@^6.5.2, qs@^6.6.0: +qs@^6.5.2: version "6.9.3" resolved "https://registry.yarnpkg.com/qs/-/qs-6.9.3.tgz#bfadcd296c2d549f1dffa560619132c977f5008e" integrity sha512-EbZYNarm6138UKKq46tdx08Yo/q9ZhFoAXAI1meAFd2GtbRDhbZY2WQSICskT0c5q99aFzLG1D4nvTk9tqfXIw== @@ -7101,7 +6948,7 @@ randomfill@^1.0.3: randombytes "^2.0.5" safe-buffer "^5.1.0" -rc@1.2.8, rc@^1.2.7, rc@^1.2.8: +rc@1.2.8, rc@^1.2.8: version "1.2.8" resolved "https://registry.yarnpkg.com/rc/-/rc-1.2.8.tgz#cd924bf5200a075b83c188cd6b9e211b7fc0d3ed" integrity sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw== @@ -7259,7 +7106,7 @@ read-pkg@^4.0.1: parse-json "^4.0.0" pify "^3.0.0" -"readable-stream@1 || 2", readable-stream@^2.0.0, readable-stream@^2.0.1, readable-stream@^2.0.2, readable-stream@^2.0.6, readable-stream@^2.1.5, readable-stream@^2.2.2, readable-stream@^2.3.3, readable-stream@^2.3.6, readable-stream@~2.3.6: +"readable-stream@1 || 2", readable-stream@^2.0.0, readable-stream@^2.0.1, readable-stream@^2.0.2, readable-stream@^2.1.5, readable-stream@^2.2.2, readable-stream@^2.3.3, readable-stream@^2.3.6, readable-stream@~2.3.6: version "2.3.7" resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-2.3.7.tgz#1eca1cf711aef814c04f62252a36a62f6cb23b57" integrity sha512-Ebho8K4jIbHAxnuxi7o42OrZgF/ZTNcsZj6nRKyUmkhLFq8CHItp/fy6hQZuZmP/n3yZ9VBUbp4zz/mX8hmYPw== @@ -7619,7 +7466,7 @@ rimraf@2.6.3: dependencies: glob "^7.1.3" -rimraf@^2.4.3, rimraf@^2.5.4, rimraf@^2.6.1, rimraf@^2.6.3: +rimraf@^2.4.3, rimraf@^2.5.4, rimraf@^2.6.3: version "2.7.1" resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.7.1.tgz#35797f13a7fdadc566142c29d4f07ccad483e3ec" integrity sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w== @@ -7781,7 +7628,7 @@ serialize-javascript@^2.1.2: resolved "https://registry.yarnpkg.com/serialize-javascript/-/serialize-javascript-2.1.2.tgz#ecec53b0e0317bdc95ef76ab7074b7384785fa61" integrity sha512-rs9OggEUF0V4jUSecXazOYsLfu7OGK2qIn3c7IPBiffz32XniEp/TX9Xmc9LQfK2nQ2QKHvZ2oygKUGU0lG4jQ== -set-blocking@^2.0.0, set-blocking@~2.0.0: +set-blocking@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/set-blocking/-/set-blocking-2.0.0.tgz#045f9782d011ae9a6803ddd382b24392b3d890f7" integrity sha1-BF+XgtARrppoA93TgrJDkrPYkPc= @@ -8117,7 +7964,7 @@ string-width@^1.0.1: is-fullwidth-code-point "^1.0.0" strip-ansi "^3.0.0" -"string-width@^1.0.2 || 2", string-width@^2.0.0, string-width@^2.1.0, string-width@^2.1.1: +string-width@^2.0.0, string-width@^2.1.0, string-width@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/string-width/-/string-width-2.1.1.tgz#ab93f27a8dc13d28cac815c462143a6d9012ae9e" integrity sha512-nOqH59deCq9SRHlxq1Aw85Jnt4w6KvLKqWVik6oA9ZklXLNIOlqg4F2yrT1MVaTjAqvVwdfeZ7w7aCvJD7ugkw== @@ -8398,19 +8245,6 @@ tapable@^1.0.0, tapable@^1.1.3: resolved "https://registry.yarnpkg.com/tapable/-/tapable-1.1.3.tgz#a1fccc06b58db61fd7a45da2da44f5f3a3e67ba2" integrity sha512-4WK/bYZmj8xLr+HUCODHGF1ZFzsYffasLUgEiMBY4fgtltdO6B4WJtlSbPaDTLpYTcGVwM2qLnFTICEcNxs3kA== -tar@^4.4.2: - version "4.4.13" - resolved "https://registry.yarnpkg.com/tar/-/tar-4.4.13.tgz#43b364bc52888d555298637b10d60790254ab525" - integrity sha512-w2VwSrBoHa5BsSyH+KxEqeQBAllHhccyMFVHtGtdMpF4W7IRWfZjFiQceJPChOeTsSDVUpER2T8FA93pr0L+QA== - dependencies: - chownr "^1.1.1" - fs-minipass "^1.2.5" - minipass "^2.8.6" - minizlib "^1.2.1" - mkdirp "^0.5.0" - safe-buffer "^5.1.2" - yallist "^3.0.3" - terser-webpack-plugin@^1.4.3: version "1.4.3" resolved "https://registry.yarnpkg.com/terser-webpack-plugin/-/terser-webpack-plugin-1.4.3.tgz#5ecaf2dbdc5fb99745fd06791f46fc9ddb1c9a7c" @@ -9133,13 +8967,6 @@ which@^1.2.14, which@^1.2.9, which@^1.3.0, which@^1.3.1: dependencies: isexe "^2.0.0" -wide-align@^1.1.0: - version "1.1.3" - resolved "https://registry.yarnpkg.com/wide-align/-/wide-align-1.1.3.tgz#ae074e6bdc0c14a431e804e624549c633b000457" - integrity sha512-QGkOQc8XL6Bt5PwnsExKBPuMKBxnGxWWW3fU55Xt4feHozMUhdUMaBCk290qpm/wG5u/RSKzwdAC4i51YigihA== - dependencies: - string-width "^1.0.2 || 2" - word-wrap@~1.2.3: version "1.2.3" resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.3.tgz#610636f6b1f703891bd34771ccb17fb93b47079c" @@ -9224,7 +9051,7 @@ xtend@^4.0.0, xtend@^4.0.1, xtend@~4.0.1: resolved "https://registry.yarnpkg.com/y18n/-/y18n-4.0.0.tgz#95ef94f85ecc81d007c264e190a120f0a3c8566b" integrity sha512-r9S/ZyXu/Xu9q1tYlpsLIsa3EeLXXk0VwlxqTcFRfg9EhMW+17kbt9G0NrgCmhGb5vT2hyhJZLfDGx+7+5Uj/w== -yallist@^3.0.0, yallist@^3.0.2, yallist@^3.0.3: +yallist@^3.0.2: version "3.1.1" resolved "https://registry.yarnpkg.com/yallist/-/yallist-3.1.1.tgz#dbb7daf9bfd8bac9ab45ebf602b8cbad0d5d08fd" integrity sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g== From 32cc48ff7a714fab5ef802cf4e94358475ebe73b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 08:49:19 +0100 Subject: [PATCH 3/8] Fix issue with room duplication caused by filtering and selecting room using keyboard Wrap sticky room updates in lock to prevent setStickyRoom running in middle of setKnownRooms --- src/stores/room-list/algorithms/Algorithm.ts | 147 ++++++++++--------- 1 file changed, 80 insertions(+), 67 deletions(-) diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index f50d112248..2acce1ecd7 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -16,8 +16,10 @@ limitations under the License. import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; -import DMRoomMap from "../../../utils/DMRoomMap"; import { EventEmitter } from "events"; +import AwaitLock from "await-lock"; + +import DMRoomMap from "../../../utils/DMRoomMap"; import { arrayDiff, arrayHasDiff } from "../../../utils/arrays"; import { DefaultTagID, RoomUpdateCause, TagID } from "../models"; import { @@ -78,6 +80,7 @@ export class Algorithm extends EventEmitter { } = {}; private allowedByFilter: Map = new Map(); private allowedRoomsByFilters: Set = new Set(); + private stickyLock = new AwaitLock(); /** * Set to true to suspend emissions of algorithm updates. @@ -123,7 +126,12 @@ export class Algorithm extends EventEmitter { * @param val The new room to sticky. */ public async setStickyRoom(val: Room) { - await this.updateStickyRoom(val); + await this.stickyLock.acquireAsync(); + try { + await this.updateStickyRoom(val); + } finally { + this.stickyLock.release(); + } } public getTagSorting(tagId: TagID): SortAlgorithm { @@ -519,82 +527,87 @@ export class Algorithm extends EventEmitter { if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); - if (!this.updatesInhibited) { - // We only log this if we're expecting to be publishing updates, which means that - // this could be an unexpected invocation. If we're inhibited, then this is probably - // an intentional invocation. - console.warn("Resetting known rooms, initiating regeneration"); - } + await this.stickyLock.acquireAsync(); + try { + if (!this.updatesInhibited) { + // We only log this if we're expecting to be publishing updates, which means that + // this could be an unexpected invocation. If we're inhibited, then this is probably + // an intentional invocation. + console.warn("Resetting known rooms, initiating regeneration"); + } - // Before we go any further we need to clear (but remember) the sticky room to - // avoid accidentally duplicating it in the list. - const oldStickyRoom = this._stickyRoom; - await this.updateStickyRoom(null); + // Before we go any further we need to clear (but remember) the sticky room to + // avoid accidentally duplicating it in the list. + const oldStickyRoom = this._stickyRoom; + if (oldStickyRoom) await this.updateStickyRoom(null); - this.rooms = rooms; + this.rooms = rooms; - const newTags: ITagMap = {}; - for (const tagId in this.sortAlgorithms) { - // noinspection JSUnfilteredForInLoop - newTags[tagId] = []; - } + const newTags: ITagMap = {}; + for (const tagId in this.sortAlgorithms) { + // noinspection JSUnfilteredForInLoop + newTags[tagId] = []; + } - // If we can avoid doing work, do so. - if (!rooms.length) { - await this.generateFreshTags(newTags); // just in case it wants to do something - this.cachedRooms = newTags; - return; - } + // If we can avoid doing work, do so. + if (!rooms.length) { + await this.generateFreshTags(newTags); // just in case it wants to do something + this.cachedRooms = newTags; + return; + } - // Split out the easy rooms first (leave and invite) - const memberships = splitRoomsByMembership(rooms); - for (const room of memberships[EffectiveMembership.Invite]) { - newTags[DefaultTagID.Invite].push(room); - } - for (const room of memberships[EffectiveMembership.Leave]) { - newTags[DefaultTagID.Archived].push(room); - } + // Split out the easy rooms first (leave and invite) + const memberships = splitRoomsByMembership(rooms); + for (const room of memberships[EffectiveMembership.Invite]) { + newTags[DefaultTagID.Invite].push(room); + } + for (const room of memberships[EffectiveMembership.Leave]) { + newTags[DefaultTagID.Archived].push(room); + } - // Now process all the joined rooms. This is a bit more complicated - for (const room of memberships[EffectiveMembership.Join]) { - const tags = this.getTagsOfJoinedRoom(room); + // Now process all the joined rooms. This is a bit more complicated + for (const room of memberships[EffectiveMembership.Join]) { + const tags = this.getTagsOfJoinedRoom(room); - let inTag = false; - if (tags.length > 0) { - for (const tag of tags) { - if (!isNullOrUndefined(newTags[tag])) { - newTags[tag].push(room); - inTag = true; + let inTag = false; + if (tags.length > 0) { + for (const tag of tags) { + if (!isNullOrUndefined(newTags[tag])) { + newTags[tag].push(room); + inTag = true; + } + } + } + + if (!inTag) { + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + newTags[DefaultTagID.DM].push(room); + } else { + newTags[DefaultTagID.Untagged].push(room); } } } - if (!inTag) { - if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { - newTags[DefaultTagID.DM].push(room); - } else { - newTags[DefaultTagID.Untagged].push(room); - } - } - } - - await this.generateFreshTags(newTags); - - this.cachedRooms = newTags; // this recalculates the filtered rooms for us - this.updateTagsFromCache(); - - // Now that we've finished generation, we need to update the sticky room to what - // it was. It's entirely possible that it changed lists though, so if it did then - // we also have to update the position of it. - if (oldStickyRoom && oldStickyRoom.room) { - await this.updateStickyRoom(oldStickyRoom.room); - if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan - if (this._stickyRoom.tag !== oldStickyRoom.tag) { - // We put the sticky room at the top of the list to treat it as an obvious tag change. - this._stickyRoom.position = 0; - this.recalculateStickyRoom(this._stickyRoom.tag); + await this.generateFreshTags(newTags); + + this.cachedRooms = newTags; // this recalculates the filtered rooms for us + this.updateTagsFromCache(); + + // Now that we've finished generation, we need to update the sticky room to what + // it was. It's entirely possible that it changed lists though, so if it did then + // we also have to update the position of it. + if (oldStickyRoom && oldStickyRoom.room) { + await this.updateStickyRoom(oldStickyRoom.room); + if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan + if (this._stickyRoom.tag !== oldStickyRoom.tag) { + // We put the sticky room at the top of the list to treat it as an obvious tag change. + this._stickyRoom.position = 0; + this.recalculateStickyRoom(this._stickyRoom.tag); + } } } + } finally { + this.stickyLock.release(); } } @@ -685,9 +698,9 @@ export class Algorithm extends EventEmitter { if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); // Note: check the isSticky against the room ID just in case the reference is wrong - const isSticky = this._stickyRoom && this._stickyRoom.room && this._stickyRoom.room.roomId === room.roomId; + const isSticky = this._stickyRoom?.room?.roomId === room.roomId; if (cause === RoomUpdateCause.NewRoom) { - const isForLastSticky = this._lastStickyRoom && this._lastStickyRoom.room === room; + const isForLastSticky = this._lastStickyRoom?.room === room; const roomTags = this.roomIdsToTags[room.roomId]; const hasTags = roomTags && roomTags.length > 0; From 191591e80730990c18f9d24c35273cfa2c2fb18d Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 09:15:56 +0100 Subject: [PATCH 4/8] Make the critical sections of the RLS synchronous to avoid needing to mutex everything --- src/components/views/rooms/RoomSublist.tsx | 4 +- src/stores/room-list/RoomListStore.ts | 54 ++--- src/stores/room-list/algorithms/Algorithm.ts | 200 ++++++++---------- .../list-ordering/ImportanceAlgorithm.ts | 98 ++++----- .../list-ordering/NaturalAlgorithm.ts | 58 +++-- .../list-ordering/OrderingAlgorithm.ts | 10 +- .../tag-sorting/AlphabeticAlgorithm.ts | 2 +- .../algorithms/tag-sorting/IAlgorithm.ts | 4 +- .../algorithms/tag-sorting/ManualAlgorithm.ts | 2 +- .../algorithms/tag-sorting/RecentAlgorithm.ts | 2 +- .../room-list/algorithms/tag-sorting/index.ts | 4 +- 11 files changed, 199 insertions(+), 239 deletions(-) diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index fce9e297a1..8d825a2b53 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -408,10 +408,10 @@ export default class RoomSublist extends React.Component { this.setState({ addRoomContextMenuPosition: null }); }; - private onUnreadFirstChanged = async () => { + private onUnreadFirstChanged = () => { const isUnreadFirst = RoomListStore.instance.getListOrder(this.props.tagId) === ListAlgorithm.Importance; const newAlgorithm = isUnreadFirst ? ListAlgorithm.Natural : ListAlgorithm.Importance; - await RoomListStore.instance.setListOrder(this.props.tagId, newAlgorithm); + RoomListStore.instance.setListOrder(this.props.tagId, newAlgorithm); this.forceUpdate(); // because if the sublist doesn't have any changes then we will miss the list order change }; diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index bedbfebd7f..3913a2220f 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -132,8 +132,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient { // Update any settings here, as some may have happened before we were logically ready. console.log("Regenerating room lists: Startup"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists({ trigger: false }); - await this.handleRVSUpdate({ trigger: false }); // fake an RVS update to adjust sticky room, if needed + this.regenerateAllLists({ trigger: false }); + this.handleRVSUpdate({ trigger: false }); // fake an RVS update to adjust sticky room, if needed this.updateFn.mark(); // we almost certainly want to trigger an update. this.updateFn.trigger(); @@ -150,7 +150,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { await this.updateState({ tagsEnabled, }); - await this.updateAlgorithmInstances(); + this.updateAlgorithmInstances(); } /** @@ -158,23 +158,23 @@ export class RoomListStoreClass extends AsyncStoreWithClient { * @param trigger Set to false to prevent a list update from being sent. Should only * be used if the calling code will manually trigger the update. */ - private async handleRVSUpdate({ trigger = true }) { + private handleRVSUpdate({ trigger = true }) { if (!this.matrixClient) return; // We assume there won't be RVS updates without a client const activeRoomId = RoomViewStore.getRoomId(); if (!activeRoomId && this.algorithm.stickyRoom) { - await this.algorithm.setStickyRoom(null); + this.algorithm.setStickyRoom(null); } else if (activeRoomId) { const activeRoom = this.matrixClient.getRoom(activeRoomId); if (!activeRoom) { console.warn(`${activeRoomId} is current in RVS but missing from client - clearing sticky room`); - await this.algorithm.setStickyRoom(null); + this.algorithm.setStickyRoom(null); } else if (activeRoom !== this.algorithm.stickyRoom) { if (SettingsStore.getValue("advancedRoomListLogging")) { // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 console.log(`Changing sticky room to ${activeRoomId}`); } - await this.algorithm.setStickyRoom(activeRoom); + this.algorithm.setStickyRoom(activeRoom); } } @@ -226,7 +226,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { console.log("Regenerating room lists: Settings changed"); await this.readAndCacheSettingsFromStore(); - await this.regenerateAllLists({ trigger: false }); // regenerate the lists now + this.regenerateAllLists({ trigger: false }); // regenerate the lists now this.updateFn.trigger(); } } @@ -368,7 +368,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 console.log(`[RoomListDebug] Clearing sticky room due to room upgrade`); } - await this.algorithm.setStickyRoom(null); + this.algorithm.setStickyRoom(null); } // Note: we hit the algorithm instead of our handleRoomUpdate() function to @@ -377,7 +377,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 console.log(`[RoomListDebug] Removing previous room from room list`); } - await this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved); + this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved); } } @@ -433,7 +433,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { return; // don't do anything on new/moved rooms which ought not to be shown } - const shouldUpdate = await this.algorithm.handleRoomUpdate(room, cause); + const shouldUpdate = this.algorithm.handleRoomUpdate(room, cause); if (shouldUpdate) { if (SettingsStore.getValue("advancedRoomListLogging")) { // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 @@ -462,13 +462,13 @@ export class RoomListStoreClass extends AsyncStoreWithClient { // Reset the sticky room before resetting the known rooms so the algorithm // doesn't freak out. - await this.algorithm.setStickyRoom(null); - await this.algorithm.setKnownRooms(rooms); + this.algorithm.setStickyRoom(null); + this.algorithm.setKnownRooms(rooms); // Set the sticky room back, if needed, now that we have updated the store. // This will use relative stickyness to the new room set. if (stickyIsStillPresent) { - await this.algorithm.setStickyRoom(currentSticky); + this.algorithm.setStickyRoom(currentSticky); } // Finally, mark an update and resume updates from the algorithm @@ -477,12 +477,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient { } public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { - await this.setAndPersistTagSorting(tagId, sort); + this.setAndPersistTagSorting(tagId, sort); this.updateFn.trigger(); } - private async setAndPersistTagSorting(tagId: TagID, sort: SortAlgorithm) { - await this.algorithm.setTagSorting(tagId, sort); + private setAndPersistTagSorting(tagId: TagID, sort: SortAlgorithm) { + this.algorithm.setTagSorting(tagId, sort); // TODO: Per-account? https://github.com/vector-im/element-web/issues/14114 localStorage.setItem(`mx_tagSort_${tagId}`, sort); } @@ -520,13 +520,13 @@ export class RoomListStoreClass extends AsyncStoreWithClient { return tagSort; } - public async setListOrder(tagId: TagID, order: ListAlgorithm) { - await this.setAndPersistListOrder(tagId, order); + public setListOrder(tagId: TagID, order: ListAlgorithm) { + this.setAndPersistListOrder(tagId, order); this.updateFn.trigger(); } - private async setAndPersistListOrder(tagId: TagID, order: ListAlgorithm) { - await this.algorithm.setListOrdering(tagId, order); + private setAndPersistListOrder(tagId: TagID, order: ListAlgorithm) { + this.algorithm.setListOrdering(tagId, order); // TODO: Per-account? https://github.com/vector-im/element-web/issues/14114 localStorage.setItem(`mx_listOrder_${tagId}`, order); } @@ -563,7 +563,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { return listOrder; } - private async updateAlgorithmInstances() { + private updateAlgorithmInstances() { // We'll require an update, so mark for one. Marking now also prevents the calls // to setTagSorting and setListOrder from causing triggers. this.updateFn.mark(); @@ -576,10 +576,10 @@ export class RoomListStoreClass extends AsyncStoreWithClient { const listOrder = this.calculateListOrder(tag); if (tagSort !== definedSort) { - await this.setAndPersistTagSorting(tag, tagSort); + this.setAndPersistTagSorting(tag, tagSort); } if (listOrder !== definedOrder) { - await this.setAndPersistListOrder(tag, listOrder); + this.setAndPersistListOrder(tag, listOrder); } } } @@ -632,7 +632,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient { * @param trigger Set to false to prevent a list update from being sent. Should only * be used if the calling code will manually trigger the update. */ - public async regenerateAllLists({ trigger = true }) { + public regenerateAllLists({ trigger = true }) { console.warn("Regenerating all room lists"); const rooms = this.getPlausibleRooms(); @@ -656,8 +656,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient { RoomListLayoutStore.instance.ensureLayoutExists(tagId); } - await this.algorithm.populateTags(sorts, orders); - await this.algorithm.setKnownRooms(rooms); + this.algorithm.populateTags(sorts, orders); + this.algorithm.setKnownRooms(rooms); this.initialListsGenerated = true; diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 2acce1ecd7..8574f095d6 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -17,7 +17,6 @@ limitations under the License. import { Room } from "matrix-js-sdk/src/models/room"; import { isNullOrUndefined } from "matrix-js-sdk/src/utils"; import { EventEmitter } from "events"; -import AwaitLock from "await-lock"; import DMRoomMap from "../../../utils/DMRoomMap"; import { arrayDiff, arrayHasDiff } from "../../../utils/arrays"; @@ -80,7 +79,6 @@ export class Algorithm extends EventEmitter { } = {}; private allowedByFilter: Map = new Map(); private allowedRoomsByFilters: Set = new Set(); - private stickyLock = new AwaitLock(); /** * Set to true to suspend emissions of algorithm updates. @@ -125,13 +123,8 @@ export class Algorithm extends EventEmitter { * Awaitable version of the sticky room setter. * @param val The new room to sticky. */ - public async setStickyRoom(val: Room) { - await this.stickyLock.acquireAsync(); - try { - await this.updateStickyRoom(val); - } finally { - this.stickyLock.release(); - } + public setStickyRoom(val: Room) { + this.updateStickyRoom(val); } public getTagSorting(tagId: TagID): SortAlgorithm { @@ -139,13 +132,13 @@ export class Algorithm extends EventEmitter { return this.sortAlgorithms[tagId]; } - public async setTagSorting(tagId: TagID, sort: SortAlgorithm) { + public setTagSorting(tagId: TagID, sort: SortAlgorithm) { if (!tagId) throw new Error("Tag ID must be defined"); if (!sort) throw new Error("Algorithm must be defined"); this.sortAlgorithms[tagId] = sort; const algorithm: OrderingAlgorithm = this.algorithms[tagId]; - await algorithm.setSortAlgorithm(sort); + algorithm.setSortAlgorithm(sort); this._cachedRooms[tagId] = algorithm.orderedRooms; this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed @@ -156,7 +149,7 @@ export class Algorithm extends EventEmitter { return this.listAlgorithms[tagId]; } - public async setListOrdering(tagId: TagID, order: ListAlgorithm) { + public setListOrdering(tagId: TagID, order: ListAlgorithm) { if (!tagId) throw new Error("Tag ID must be defined"); if (!order) throw new Error("Algorithm must be defined"); this.listAlgorithms[tagId] = order; @@ -164,7 +157,7 @@ export class Algorithm extends EventEmitter { const algorithm = getListAlgorithmInstance(order, tagId, this.sortAlgorithms[tagId]); this.algorithms[tagId] = algorithm; - await algorithm.setRooms(this._cachedRooms[tagId]); + algorithm.setRooms(this._cachedRooms[tagId]); this._cachedRooms[tagId] = algorithm.orderedRooms; this.recalculateFilteredRoomsForTag(tagId); // update filter to re-sort the list this.recalculateStickyRoom(tagId); // update sticky room to make sure it appears if needed @@ -191,31 +184,25 @@ export class Algorithm extends EventEmitter { } } - private async handleFilterChange() { - await this.recalculateFilteredRooms(); + private handleFilterChange() { + this.recalculateFilteredRooms(); // re-emit the update so the list store can fire an off-cycle update if needed if (this.updatesInhibited) return; this.emit(FILTER_CHANGED); } - private async updateStickyRoom(val: Room) { - try { - return await this.doUpdateStickyRoom(val); - } finally { - this._lastStickyRoom = null; // clear to indicate we're done changing - } + private updateStickyRoom(val: Room) { + this.doUpdateStickyRoom(val); + this._lastStickyRoom = null; // clear to indicate we're done changing } - private async doUpdateStickyRoom(val: Room) { + private doUpdateStickyRoom(val: Room) { if (SpaceStore.spacesEnabled && val?.isSpaceRoom() && val.getMyMembership() !== "invite") { // no-op sticky rooms for spaces - they're effectively virtual rooms val = null; } - // Note throughout: We need async so we can wait for handleRoomUpdate() to do its thing, - // otherwise we risk duplicating rooms. - if (val && !VisibilityProvider.instance.isRoomVisible(val)) { val = null; // the room isn't visible - lie to the rest of this function } @@ -231,7 +218,7 @@ export class Algorithm extends EventEmitter { this._stickyRoom = null; // clear before we go to update the algorithm // Lie to the algorithm and re-add the room to the algorithm - await this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); + this.handleRoomUpdate(stickyRoom, RoomUpdateCause.NewRoom); return; } return; @@ -277,10 +264,10 @@ export class Algorithm extends EventEmitter { // referential checks as the references can differ through the lifecycle. if (lastStickyRoom && lastStickyRoom.room && lastStickyRoom.room.roomId !== val.roomId) { // Lie to the algorithm and re-add the room to the algorithm - await this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom); + this.handleRoomUpdate(lastStickyRoom.room, RoomUpdateCause.NewRoom); } // Lie to the algorithm and remove the room from it's field of view - await this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved); + this.handleRoomUpdate(val, RoomUpdateCause.RoomRemoved); // Check for tag & position changes while we're here. We also check the room to ensure // it is still the same room. @@ -470,9 +457,8 @@ export class Algorithm extends EventEmitter { * them. * @param {ITagSortingMap} tagSortingMap The tags to generate. * @param {IListOrderingMap} listOrderingMap The ordering of those tags. - * @returns {Promise<*>} A promise which resolves when complete. */ - public async populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): Promise { + public populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): void { if (!tagSortingMap) throw new Error(`Sorting map cannot be null or empty`); if (!listOrderingMap) throw new Error(`Ordering ma cannot be null or empty`); if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) { @@ -521,93 +507,87 @@ export class Algorithm extends EventEmitter { * Seeds the Algorithm with a set of rooms. The algorithm will discard all * previously known information and instead use these rooms instead. * @param {Room[]} rooms The rooms to force the algorithm to use. - * @returns {Promise<*>} A promise which resolves when complete. */ - public async setKnownRooms(rooms: Room[]): Promise { + public setKnownRooms(rooms: Room[]): void { if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); - await this.stickyLock.acquireAsync(); - try { - if (!this.updatesInhibited) { - // We only log this if we're expecting to be publishing updates, which means that - // this could be an unexpected invocation. If we're inhibited, then this is probably - // an intentional invocation. - console.warn("Resetting known rooms, initiating regeneration"); - } + if (!this.updatesInhibited) { + // We only log this if we're expecting to be publishing updates, which means that + // this could be an unexpected invocation. If we're inhibited, then this is probably + // an intentional invocation. + console.warn("Resetting known rooms, initiating regeneration"); + } - // Before we go any further we need to clear (but remember) the sticky room to - // avoid accidentally duplicating it in the list. - const oldStickyRoom = this._stickyRoom; - if (oldStickyRoom) await this.updateStickyRoom(null); + // Before we go any further we need to clear (but remember) the sticky room to + // avoid accidentally duplicating it in the list. + const oldStickyRoom = this._stickyRoom; + if (oldStickyRoom) this.updateStickyRoom(null); - this.rooms = rooms; + this.rooms = rooms; - const newTags: ITagMap = {}; - for (const tagId in this.sortAlgorithms) { - // noinspection JSUnfilteredForInLoop - newTags[tagId] = []; - } + const newTags: ITagMap = {}; + for (const tagId in this.sortAlgorithms) { + // noinspection JSUnfilteredForInLoop + newTags[tagId] = []; + } - // If we can avoid doing work, do so. - if (!rooms.length) { - await this.generateFreshTags(newTags); // just in case it wants to do something - this.cachedRooms = newTags; - return; - } + // If we can avoid doing work, do so. + if (!rooms.length) { + this.generateFreshTags(newTags); // just in case it wants to do something + this.cachedRooms = newTags; + return; + } - // Split out the easy rooms first (leave and invite) - const memberships = splitRoomsByMembership(rooms); - for (const room of memberships[EffectiveMembership.Invite]) { - newTags[DefaultTagID.Invite].push(room); - } - for (const room of memberships[EffectiveMembership.Leave]) { - newTags[DefaultTagID.Archived].push(room); - } + // Split out the easy rooms first (leave and invite) + const memberships = splitRoomsByMembership(rooms); + for (const room of memberships[EffectiveMembership.Invite]) { + newTags[DefaultTagID.Invite].push(room); + } + for (const room of memberships[EffectiveMembership.Leave]) { + newTags[DefaultTagID.Archived].push(room); + } - // Now process all the joined rooms. This is a bit more complicated - for (const room of memberships[EffectiveMembership.Join]) { - const tags = this.getTagsOfJoinedRoom(room); + // Now process all the joined rooms. This is a bit more complicated + for (const room of memberships[EffectiveMembership.Join]) { + const tags = this.getTagsOfJoinedRoom(room); - let inTag = false; - if (tags.length > 0) { - for (const tag of tags) { - if (!isNullOrUndefined(newTags[tag])) { - newTags[tag].push(room); - inTag = true; - } - } - } - - if (!inTag) { - if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { - newTags[DefaultTagID.DM].push(room); - } else { - newTags[DefaultTagID.Untagged].push(room); + let inTag = false; + if (tags.length > 0) { + for (const tag of tags) { + if (!isNullOrUndefined(newTags[tag])) { + newTags[tag].push(room); + inTag = true; } } } - await this.generateFreshTags(newTags); - - this.cachedRooms = newTags; // this recalculates the filtered rooms for us - this.updateTagsFromCache(); - - // Now that we've finished generation, we need to update the sticky room to what - // it was. It's entirely possible that it changed lists though, so if it did then - // we also have to update the position of it. - if (oldStickyRoom && oldStickyRoom.room) { - await this.updateStickyRoom(oldStickyRoom.room); - if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan - if (this._stickyRoom.tag !== oldStickyRoom.tag) { - // We put the sticky room at the top of the list to treat it as an obvious tag change. - this._stickyRoom.position = 0; - this.recalculateStickyRoom(this._stickyRoom.tag); - } + if (!inTag) { + if (DMRoomMap.shared().getUserIdForRoomId(room.roomId)) { + newTags[DefaultTagID.DM].push(room); + } else { + newTags[DefaultTagID.Untagged].push(room); + } + } + } + + this.generateFreshTags(newTags); + + this.cachedRooms = newTags; // this recalculates the filtered rooms for us + this.updateTagsFromCache(); + + // Now that we've finished generation, we need to update the sticky room to what + // it was. It's entirely possible that it changed lists though, so if it did then + // we also have to update the position of it. + if (oldStickyRoom && oldStickyRoom.room) { + this.updateStickyRoom(oldStickyRoom.room); + if (this._stickyRoom && this._stickyRoom.room) { // just in case the update doesn't go according to plan + if (this._stickyRoom.tag !== oldStickyRoom.tag) { + // We put the sticky room at the top of the list to treat it as an obvious tag change. + this._stickyRoom.position = 0; + this.recalculateStickyRoom(this._stickyRoom.tag); } } - } finally { - this.stickyLock.release(); } } @@ -665,16 +645,15 @@ export class Algorithm extends EventEmitter { * @param {ITagMap} updatedTagMap The tag map which needs populating. Each tag * will already have the rooms which belong to it - they just need ordering. Must * be mutated in place. - * @returns {Promise<*>} A promise which resolves when complete. */ - private async generateFreshTags(updatedTagMap: ITagMap): Promise { + private generateFreshTags(updatedTagMap: ITagMap): void { if (!this.algorithms) throw new Error("Not ready: no algorithms to determine tags from"); for (const tag of Object.keys(updatedTagMap)) { const algorithm: OrderingAlgorithm = this.algorithms[tag]; if (!algorithm) throw new Error(`No algorithm for ${tag}`); - await algorithm.setRooms(updatedTagMap[tag]); + algorithm.setRooms(updatedTagMap[tag]); updatedTagMap[tag] = algorithm.orderedRooms; } } @@ -686,11 +665,10 @@ export class Algorithm extends EventEmitter { * may no-op this request if no changes are required. * @param {Room} room The room which might have affected sorting. * @param {RoomUpdateCause} cause The reason for the update being triggered. - * @returns {Promise} A promise which resolve to true or false - * depending on whether or not getOrderedRooms() should be called after - * processing. + * @returns {Promise} A boolean of whether or not getOrderedRooms() + * should be called after processing. */ - public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { + public handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean { if (SettingsStore.getValue("advancedRoomListLogging")) { // TODO: Remove debug: https://github.com/vector-im/element-web/issues/14602 console.log(`Handle room update for ${room.roomId} called with cause ${cause}`); @@ -757,7 +735,7 @@ export class Algorithm extends EventEmitter { } const algorithm: OrderingAlgorithm = this.algorithms[rmTag]; if (!algorithm) throw new Error(`No algorithm for ${rmTag}`); - await algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); + algorithm.handleRoomUpdate(room, RoomUpdateCause.RoomRemoved); this._cachedRooms[rmTag] = algorithm.orderedRooms; this.recalculateFilteredRoomsForTag(rmTag); // update filter to re-sort the list this.recalculateStickyRoom(rmTag); // update sticky room to make sure it moves if needed @@ -769,7 +747,7 @@ export class Algorithm extends EventEmitter { } const algorithm: OrderingAlgorithm = this.algorithms[addTag]; if (!algorithm) throw new Error(`No algorithm for ${addTag}`); - await algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); + algorithm.handleRoomUpdate(room, RoomUpdateCause.NewRoom); this._cachedRooms[addTag] = algorithm.orderedRooms; } @@ -802,7 +780,7 @@ export class Algorithm extends EventEmitter { }; } else { // We have to clear the lock as the sticky room change will trigger updates. - await this.setStickyRoom(room); + this.setStickyRoom(room); } } } @@ -865,7 +843,7 @@ export class Algorithm extends EventEmitter { const algorithm: OrderingAlgorithm = this.algorithms[tag]; if (!algorithm) throw new Error(`No algorithm for ${tag}`); - await algorithm.handleRoomUpdate(room, cause); + algorithm.handleRoomUpdate(room, cause); this._cachedRooms[tag] = algorithm.orderedRooms; // Flag that we've done something diff --git a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts index 80bdf74afb..1d35df331d 100644 --- a/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/ImportanceAlgorithm.ts @@ -94,15 +94,15 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { return state.color; } - public async setRooms(rooms: Room[]): Promise { + public setRooms(rooms: Room[]): void { if (this.sortingAlgorithm === SortAlgorithm.Manual) { - this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); + this.cachedOrderedRooms = sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); } else { // Every other sorting type affects the categories, not the whole tag. const categorized = this.categorizeRooms(rooms); for (const category of Object.keys(categorized)) { const roomsToOrder = categorized[category]; - categorized[category] = await sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm); + categorized[category] = sortRoomsWithAlgorithm(roomsToOrder, this.tagId, this.sortingAlgorithm); } const newlyOrganized: Room[] = []; @@ -118,12 +118,12 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { } } - private async handleSplice(room: Room, cause: RoomUpdateCause): Promise { + private handleSplice(room: Room, cause: RoomUpdateCause): boolean { if (cause === RoomUpdateCause.NewRoom) { const category = this.getRoomCategory(room); this.alterCategoryPositionBy(category, 1, this.indices); this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) - await this.sortCategory(category); + this.sortCategory(category); } else if (cause === RoomUpdateCause.RoomRemoved) { const roomIdx = this.getRoomIndex(room); if (roomIdx === -1) { @@ -141,55 +141,49 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { return true; } - public async handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise { - try { - await this.updateLock.acquireAsync(); - - if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) { - return this.handleSplice(room, cause); - } - - if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { - throw new Error(`Unsupported update cause: ${cause}`); - } - - const category = this.getRoomCategory(room); - if (this.sortingAlgorithm === SortAlgorithm.Manual) { - return; // Nothing to do here. - } - - const roomIdx = this.getRoomIndex(room); - if (roomIdx === -1) { - throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); - } - - // Try to avoid doing array operations if we don't have to: only move rooms within - // the categories if we're jumping categories - const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); - if (oldCategory !== category) { - // Move the room and update the indices - this.moveRoomIndexes(1, oldCategory, category, this.indices); - this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) - this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) - // Note: if moveRoomIndexes() is called after the splice then the insert operation - // will happen in the wrong place. Because we would have already adjusted the index - // for the category, we don't need to determine how the room is moving in the list. - // If we instead tried to insert before updating the indices, we'd have to determine - // whether the room was moving later (towards IDLE) or earlier (towards RED) from its - // current position, as it'll affect the category's start index after we remove the - // room from the array. - } - - // Sort the category now that we've dumped the room in - await this.sortCategory(category); - - return true; // change made - } finally { - await this.updateLock.release(); + public handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean { + if (cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved) { + return this.handleSplice(room, cause); } + + if (cause !== RoomUpdateCause.Timeline && cause !== RoomUpdateCause.ReadReceipt) { + throw new Error(`Unsupported update cause: ${cause}`); + } + + const category = this.getRoomCategory(room); + if (this.sortingAlgorithm === SortAlgorithm.Manual) { + return; // Nothing to do here. + } + + const roomIdx = this.getRoomIndex(room); + if (roomIdx === -1) { + throw new Error(`Room ${room.roomId} has no index in ${this.tagId}`); + } + + // Try to avoid doing array operations if we don't have to: only move rooms within + // the categories if we're jumping categories + const oldCategory = this.getCategoryFromIndices(roomIdx, this.indices); + if (oldCategory !== category) { + // Move the room and update the indices + this.moveRoomIndexes(1, oldCategory, category, this.indices); + this.cachedOrderedRooms.splice(roomIdx, 1); // splice out the old index (fixed position) + this.cachedOrderedRooms.splice(this.indices[category], 0, room); // splice in the new room (pre-adjusted) + // Note: if moveRoomIndexes() is called after the splice then the insert operation + // will happen in the wrong place. Because we would have already adjusted the index + // for the category, we don't need to determine how the room is moving in the list. + // If we instead tried to insert before updating the indices, we'd have to determine + // whether the room was moving later (towards IDLE) or earlier (towards RED) from its + // current position, as it'll affect the category's start index after we remove the + // room from the array. + } + + // Sort the category now that we've dumped the room in + this.sortCategory(category); + + return true; // change made } - private async sortCategory(category: NotificationColor) { + private sortCategory(category: NotificationColor) { // This should be relatively quick because the room is usually inserted at the top of the // category, and most popular sorting algorithms will deal with trying to keep the active // room at the top/start of the category. For the few algorithms that will have to move the @@ -201,7 +195,7 @@ export class ImportanceAlgorithm extends OrderingAlgorithm { const startIdx = this.indices[category]; const numSort = nextCategoryStartIdx - startIdx; // splice() returns up to the max, so MAX_SAFE_INT is fine const unsortedSlice = this.cachedOrderedRooms.splice(startIdx, numSort); - const sorted = await sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); + const sorted = sortRoomsWithAlgorithm(unsortedSlice, this.tagId, this.sortingAlgorithm); this.cachedOrderedRooms.splice(startIdx, 0, ...sorted); } diff --git a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts index cc2a28d892..91182dee16 100644 --- a/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/NaturalAlgorithm.ts @@ -29,42 +29,32 @@ export class NaturalAlgorithm extends OrderingAlgorithm { super(tagId, initialSortingAlgorithm); } - public async setRooms(rooms: Room[]): Promise { - this.cachedOrderedRooms = await sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); + public setRooms(rooms: Room[]): void { + this.cachedOrderedRooms = sortRoomsWithAlgorithm(rooms, this.tagId, this.sortingAlgorithm); } - public async handleRoomUpdate(room, cause): Promise { - try { - await this.updateLock.acquireAsync(); - - const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved; - const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt; - if (!isSplice && !isInPlace) { - throw new Error(`Unsupported update cause: ${cause}`); - } - - if (cause === RoomUpdateCause.NewRoom) { - this.cachedOrderedRooms.push(room); - } else if (cause === RoomUpdateCause.RoomRemoved) { - const idx = this.getRoomIndex(room); - if (idx >= 0) { - this.cachedOrderedRooms.splice(idx, 1); - } else { - console.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`); - } - } - - // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/element-web/issues/14457 - // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags - this.cachedOrderedRooms = await sortRoomsWithAlgorithm( - this.cachedOrderedRooms, - this.tagId, - this.sortingAlgorithm, - ); - - return true; - } finally { - await this.updateLock.release(); + public handleRoomUpdate(room, cause): boolean { + const isSplice = cause === RoomUpdateCause.NewRoom || cause === RoomUpdateCause.RoomRemoved; + const isInPlace = cause === RoomUpdateCause.Timeline || cause === RoomUpdateCause.ReadReceipt; + if (!isSplice && !isInPlace) { + throw new Error(`Unsupported update cause: ${cause}`); } + + if (cause === RoomUpdateCause.NewRoom) { + this.cachedOrderedRooms.push(room); + } else if (cause === RoomUpdateCause.RoomRemoved) { + const idx = this.getRoomIndex(room); + if (idx >= 0) { + this.cachedOrderedRooms.splice(idx, 1); + } else { + console.warn(`Tried to remove unknown room from ${this.tagId}: ${room.roomId}`); + } + } + + // TODO: Optimize this to avoid useless operations: https://github.com/vector-im/element-web/issues/14457 + // For example, we can skip updates to alphabetic (sometimes) and manually ordered tags + this.cachedOrderedRooms = sortRoomsWithAlgorithm(this.cachedOrderedRooms, this.tagId, this.sortingAlgorithm); + + return true; } } diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts index c47a35523c..23a8e33a41 100644 --- a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -26,7 +26,6 @@ import AwaitLock from "await-lock"; export abstract class OrderingAlgorithm { protected cachedOrderedRooms: Room[]; protected sortingAlgorithm: SortAlgorithm; - protected readonly updateLock = new AwaitLock(); protected constructor(protected tagId: TagID, initialSortingAlgorithm: SortAlgorithm) { // noinspection JSIgnoredPromiseFromCall @@ -45,21 +44,20 @@ export abstract class OrderingAlgorithm { * @param newAlgorithm The new algorithm. Must be defined. * @returns Resolves when complete. */ - public async setSortAlgorithm(newAlgorithm: SortAlgorithm) { + public setSortAlgorithm(newAlgorithm: SortAlgorithm) { if (!newAlgorithm) throw new Error("A sorting algorithm must be defined"); this.sortingAlgorithm = newAlgorithm; // Force regeneration of the rooms - await this.setRooms(this.orderedRooms); + this.setRooms(this.orderedRooms); } /** * Sets the rooms the algorithm should be handling, implying a reconstruction * of the ordering. * @param rooms The rooms to use going forward. - * @returns Resolves when complete. */ - public abstract setRooms(rooms: Room[]): Promise; + public abstract setRooms(rooms: Room[]): void; /** * Handle a room update. The Algorithm will only call this for causes which @@ -69,7 +67,7 @@ export abstract class OrderingAlgorithm { * @param cause The cause of the update. * @returns True if the update requires the Algorithm to update the presentation layers. */ - public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): Promise; + public abstract handleRoomUpdate(room: Room, cause: RoomUpdateCause): boolean; protected getRoomIndex(room: Room): number { let roomIdx = this.cachedOrderedRooms.indexOf(room); diff --git a/src/stores/room-list/algorithms/tag-sorting/AlphabeticAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/AlphabeticAlgorithm.ts index b016a4256c..45f6eaf843 100644 --- a/src/stores/room-list/algorithms/tag-sorting/AlphabeticAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/AlphabeticAlgorithm.ts @@ -23,7 +23,7 @@ import { compare } from "../../../../utils/strings"; * Sorts rooms according to the browser's determination of alphabetic. */ export class AlphabeticAlgorithm implements IAlgorithm { - public async sortRooms(rooms: Room[], tagId: TagID): Promise { + public sortRooms(rooms: Room[], tagId: TagID): Room[] { return rooms.sort((a, b) => { return compare(a.name, b.name); }); diff --git a/src/stores/room-list/algorithms/tag-sorting/IAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/IAlgorithm.ts index 6c22ee0c9c..588bbbffc9 100644 --- a/src/stores/room-list/algorithms/tag-sorting/IAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/IAlgorithm.ts @@ -25,7 +25,7 @@ export interface IAlgorithm { * Sorts the given rooms according to the sorting rules of the algorithm. * @param {Room[]} rooms The rooms to sort. * @param {TagID} tagId The tag ID in which the rooms are being sorted. - * @returns {Promise} Resolves to the sorted rooms. + * @returns {Room[]} Returns the sorted rooms. */ - sortRooms(rooms: Room[], tagId: TagID): Promise; + sortRooms(rooms: Room[], tagId: TagID): Room[]; } diff --git a/src/stores/room-list/algorithms/tag-sorting/ManualAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/ManualAlgorithm.ts index b8c0357633..9be8ba5262 100644 --- a/src/stores/room-list/algorithms/tag-sorting/ManualAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/ManualAlgorithm.ts @@ -22,7 +22,7 @@ import { IAlgorithm } from "./IAlgorithm"; * Sorts rooms according to the tag's `order` property on the room. */ export class ManualAlgorithm implements IAlgorithm { - public async sortRooms(rooms: Room[], tagId: TagID): Promise { + public sortRooms(rooms: Room[], tagId: TagID): Room[] { const getOrderProp = (r: Room) => r.tags[tagId].order || 0; return rooms.sort((a, b) => { return getOrderProp(a) - getOrderProp(b); diff --git a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts index 49cfd9e520..f47458d1b1 100644 --- a/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts +++ b/src/stores/room-list/algorithms/tag-sorting/RecentAlgorithm.ts @@ -97,7 +97,7 @@ export const sortRooms = (rooms: Room[]): Room[] => { * useful to the user. */ export class RecentAlgorithm implements IAlgorithm { - public async sortRooms(rooms: Room[], tagId: TagID): Promise { + public sortRooms(rooms: Room[], tagId: TagID): Room[] { return sortRooms(rooms); } } diff --git a/src/stores/room-list/algorithms/tag-sorting/index.ts b/src/stores/room-list/algorithms/tag-sorting/index.ts index c22865f5ba..368c76f111 100644 --- a/src/stores/room-list/algorithms/tag-sorting/index.ts +++ b/src/stores/room-list/algorithms/tag-sorting/index.ts @@ -46,8 +46,8 @@ export function getSortingAlgorithmInstance(algorithm: SortAlgorithm): IAlgorith * @param {Room[]} rooms The rooms to sort. * @param {TagID} tagId The tag in which the sorting is occurring. * @param {SortAlgorithm} algorithm The algorithm to use for sorting. - * @returns {Promise} Resolves to the sorted rooms. + * @returns {Room[]} Returns the sorted rooms. */ -export function sortRoomsWithAlgorithm(rooms: Room[], tagId: TagID, algorithm: SortAlgorithm): Promise { +export function sortRoomsWithAlgorithm(rooms: Room[], tagId: TagID, algorithm: SortAlgorithm): Room[] { return getSortingAlgorithmInstance(algorithm).sortRooms(rooms, tagId); } From 05028f1997c8731ef6a64d52e811522f61869dad Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 09:22:25 +0100 Subject: [PATCH 5/8] remove unused import --- .../room-list/algorithms/list-ordering/OrderingAlgorithm.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts index 23a8e33a41..9d7b5f9ddb 100644 --- a/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts +++ b/src/stores/room-list/algorithms/list-ordering/OrderingAlgorithm.ts @@ -17,7 +17,6 @@ limitations under the License. import { Room } from "matrix-js-sdk/src/models/room"; import { RoomUpdateCause, TagID } from "../../models"; import { SortAlgorithm } from "../models"; -import AwaitLock from "await-lock"; /** * Represents a list ordering algorithm. Subclasses should populate the From 9d45a3760fd38928543f20343d4f27a48c7dbb42 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 13:11:43 +0100 Subject: [PATCH 6/8] Fix types of the various query params dicts, arrays can be included e.g via --- src/Lifecycle.ts | 11 ++++++----- src/components/structures/MatrixChat.tsx | 16 ++++++++-------- src/components/views/elements/AppTile.js | 1 - 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 61ded93833..410124a637 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -21,6 +21,7 @@ import { createClient } from 'matrix-js-sdk/src/matrix'; import { InvalidStoreError } from "matrix-js-sdk/src/errors"; import { MatrixClient } from "matrix-js-sdk/src/client"; import { decryptAES, encryptAES, IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; +import { QueryDict } from 'matrix-js-sdk/src/utils'; import { IMatrixClientCreds, MatrixClientPeg } from './MatrixClientPeg'; import SecurityCustomisations from "./customisations/Security"; @@ -65,7 +66,7 @@ interface ILoadSessionOpts { guestIsUrl?: string; ignoreGuest?: boolean; defaultDeviceDisplayName?: string; - fragmentQueryParams?: Record; + fragmentQueryParams?: QueryDict; } /** @@ -118,8 +119,8 @@ export async function loadSession(opts: ILoadSessionOpts = {}): Promise ) { console.log("Using guest access credentials"); return doSetLoggedIn({ - userId: fragmentQueryParams.guest_user_id, - accessToken: fragmentQueryParams.guest_access_token, + userId: fragmentQueryParams.guest_user_id as string, + accessToken: fragmentQueryParams.guest_access_token as string, homeserverUrl: guestHsUrl, identityServerUrl: guestIsUrl, guest: true, @@ -173,7 +174,7 @@ export async function getStoredSessionOwner(): Promise<[string, boolean]> { * login, else false */ export function attemptTokenLogin( - queryParams: Record, + queryParams: QueryDict, defaultDeviceDisplayName?: string, fragmentAfterLogin?: string, ): Promise { @@ -198,7 +199,7 @@ export function attemptTokenLogin( homeserver, identityServer, "m.login.token", { - token: queryParams.loginToken, + token: queryParams.loginToken as string, initial_device_display_name: defaultDeviceDisplayName, }, ).then(function(creds) { diff --git a/src/components/structures/MatrixChat.tsx b/src/components/structures/MatrixChat.tsx index 15536f260d..785838ffca 100644 --- a/src/components/structures/MatrixChat.tsx +++ b/src/components/structures/MatrixChat.tsx @@ -19,7 +19,7 @@ import { createClient } from "matrix-js-sdk/src/matrix"; import { InvalidStoreError } from "matrix-js-sdk/src/errors"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; -import { sleep, defer, IDeferred } from "matrix-js-sdk/src/utils"; +import { sleep, defer, IDeferred, QueryDict } from "matrix-js-sdk/src/utils"; // focus-visible is a Polyfill for the :focus-visible CSS pseudo-attribute used by _AccessibleButton.scss import 'focus-visible'; @@ -155,7 +155,7 @@ const ONBOARDING_FLOW_STARTERS = [ interface IScreen { screen: string; - params?: object; + params?: QueryDict; } /* eslint-disable camelcase */ @@ -185,9 +185,9 @@ interface IProps { // TODO type things better onNewScreen: (screen: string, replaceLast: boolean) => void; enableGuest?: boolean; // the queryParams extracted from the [real] query-string of the URI - realQueryParams?: Record; + realQueryParams?: QueryDict; // the initial queryParams extracted from the hash-fragment of the URI - startingFragmentQueryParams?: Record; + startingFragmentQueryParams?: QueryDict; // called when we have completed a token login onTokenLoginCompleted?: () => void; // Represents the screen to display as a result of parsing the initial window.location @@ -195,7 +195,7 @@ interface IProps { // TODO type things better // displayname, if any, to set on the device when logging in/registering. defaultDeviceDisplayName?: string; // A function that makes a registration URL - makeRegistrationUrl: (object) => string; + makeRegistrationUrl: (params: QueryDict) => string; } interface IState { @@ -298,7 +298,7 @@ export default class MatrixChat extends React.PureComponent { if (this.screenAfterLogin.screen.startsWith("room/") && params['signurl'] && params['email']) { // probably a threepid invite - try to store it const roomId = this.screenAfterLogin.screen.substring("room/".length); - ThreepidInviteStore.instance.storeInvite(roomId, params as IThreepidInviteWireFormat); + ThreepidInviteStore.instance.storeInvite(roomId, params as unknown as IThreepidInviteWireFormat); } } @@ -1952,7 +1952,7 @@ export default class MatrixChat extends React.PureComponent { this.setState({ serverConfig }); }; - private makeRegistrationUrl = (params: {[key: string]: string}) => { + private makeRegistrationUrl = (params: QueryDict) => { if (this.props.startingFragmentQueryParams.referrer) { params.referrer = this.props.startingFragmentQueryParams.referrer; } @@ -2107,7 +2107,7 @@ export default class MatrixChat extends React.PureComponent { onForgotPasswordClick={showPasswordReset ? this.onForgotPasswordClick : undefined} onServerConfigChange={this.onServerConfigChange} fragmentAfterLogin={fragmentAfterLogin} - defaultUsername={this.props.startingFragmentQueryParams.defaultUsername} + defaultUsername={this.props.startingFragmentQueryParams.defaultUsername as string} {...this.getServerProperties()} /> ); diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 1ddca61c22..7e98537180 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -39,7 +39,6 @@ import { MatrixCapabilities } from "matrix-widget-api"; import RoomWidgetContextMenu from "../context_menus/WidgetContextMenu"; import WidgetAvatar from "../avatars/WidgetAvatar"; import { replaceableComponent } from "../../../utils/replaceableComponent"; -import { urlSearchParamsToObject } from "../../../utils/UrlUtils"; @replaceableComponent("views.elements.AppTile") export default class AppTile extends React.Component { From 3b13eb7b44debf727c0ed7c75b42d0ea3c0a17b2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 13:18:12 +0100 Subject: [PATCH 7/8] Prefer URL constructor over `url` dependency --- src/HtmlUtils.tsx | 5 +---- src/utils/HostingLink.js | 10 ++-------- src/utils/UrlUtils.ts | 4 ---- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/HtmlUtils.tsx b/src/HtmlUtils.tsx index 5e83fdc2a0..a37b7f0ac9 100644 --- a/src/HtmlUtils.tsx +++ b/src/HtmlUtils.tsx @@ -25,7 +25,6 @@ import _linkifyElement from 'linkifyjs/element'; import _linkifyString from 'linkifyjs/string'; import classNames from 'classnames'; import EMOJIBASE_REGEX from 'emojibase-regex'; -import url from 'url'; import katex from 'katex'; import { AllHtmlEntities } from 'html-entities'; import { IContent } from 'matrix-js-sdk/src/models/event'; @@ -153,10 +152,8 @@ export function getHtmlText(insaneHtml: string): string { */ export function isUrlPermitted(inputUrl: string): boolean { try { - const parsed = url.parse(inputUrl); - if (!parsed.protocol) return false; // URL parser protocol includes the trailing colon - return PERMITTED_URL_SCHEMES.includes(parsed.protocol.slice(0, -1)); + return PERMITTED_URL_SCHEMES.includes(new URL(inputUrl).protocol.slice(0, -1)); } catch (e) { return false; } diff --git a/src/utils/HostingLink.js b/src/utils/HostingLink.js index 7595bdd482..134e045ca2 100644 --- a/src/utils/HostingLink.js +++ b/src/utils/HostingLink.js @@ -14,11 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -import url from 'url'; - import SdkConfig from '../SdkConfig'; import { MatrixClientPeg } from '../MatrixClientPeg'; -import { urlSearchParamsToObject } from "./UrlUtils"; export function getHostingLink(campaign) { const hostingLink = SdkConfig.get().hosting_signup_link; @@ -28,11 +25,8 @@ export function getHostingLink(campaign) { if (MatrixClientPeg.get().getDomain() !== 'matrix.org') return null; try { - const hostingUrl = url.parse(hostingLink); - const params = urlSearchParamsToObject(new URLSearchParams(hostingUrl.query)); - params.utm_campaign = campaign; - hostingUrl.search = undefined; - hostingUrl.query = params; + const hostingUrl = new URL(hostingLink); + hostingUrl.searchParams.set("utm_campaign", campaign); return hostingUrl.format(); } catch (e) { return hostingLink; diff --git a/src/utils/UrlUtils.ts b/src/utils/UrlUtils.ts index 392b44c5e9..ba43340ff5 100644 --- a/src/utils/UrlUtils.ts +++ b/src/utils/UrlUtils.ts @@ -16,10 +16,6 @@ limitations under the License. import * as url from "url"; -export function urlSearchParamsToObject(params: URLSearchParams) { - return Object.fromEntries([...params.entries()]); -} - /** * If a url has no path component, etc. abbreviate it to just the hostname * From 74bd7cad3f768be14abf57280887c4c463a19c66 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 16 Jul 2021 13:40:53 +0100 Subject: [PATCH 8/8] remove unrelated change --- src/utils/UrlUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/UrlUtils.ts b/src/utils/UrlUtils.ts index ba43340ff5..6f441ff98e 100644 --- a/src/utils/UrlUtils.ts +++ b/src/utils/UrlUtils.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import * as url from "url"; +import url from "url"; /** * If a url has no path component, etc. abbreviate it to just the hostname