From 085ab3c2dd7a8384bb26cb61b055a8ed3f506ddb Mon Sep 17 00:00:00 2001 From: Ken Sternberg <133134217+kensternberg-authentik@users.noreply.github.com> Date: Mon, 15 Jul 2024 13:36:32 -0700 Subject: [PATCH] web: all aboard the anti-if bus, according to tooling (#10220) * web: fix esbuild issue with style sheets Getting ESBuild, Lit, and Storybook to all agree on how to read and parse stylesheets is a serious pain. This fix better identifies the value types (instances) being passed from various sources in the repo to the three *different* kinds of style processors we're using (the native one, the polyfill one, and whatever the heck Storybook does internally). Falling back to using older CSS instantiating techniques one era at a time seems to do the trick. It's ugly, but in the face of the aggressive styling we use to avoid Flashes of Unstyled Content (FLoUC), it's the logic with which we're left. In standard mode, the following warning appears on the console when running a Flow: ``` Autofocus processing was blocked because a document already has a focused element. ``` In compatibility mode, the following **error** appears on the console when running a Flow: ``` crawler-inject.js:1106 Uncaught TypeError: Failed to execute 'observe' on 'MutationObserver': parameter 1 is not of type 'Node'. at initDomMutationObservers (crawler-inject.js:1106:18) at crawler-inject.js:1114:24 at Array.forEach () at initDomMutationObservers (crawler-inject.js:1114:10) at crawler-inject.js:1549:1 initDomMutationObservers @ crawler-inject.js:1106 (anonymous) @ crawler-inject.js:1114 initDomMutationObservers @ crawler-inject.js:1114 (anonymous) @ crawler-inject.js:1549 ``` Despite this error, nothing seems to be broken and flows work as anticipated. * web: all-aboard the anti-if bus, according to tooling This commit revises a number of bugs `eslint` has been complaining about for awhile now. This is the lesser of two PRs that will address this issue, and in this case the two biggest problems were inappropriate conditionals (using a `switch` for a single comparison), unnecessarily named returns, empty returns. This brings our use of conditions in-line with the coding standards we _say_ we want in eslintrc! * web: better names and logic for comparing the dates of Xliff vs generated files * Missed one. * Fixed a redirect issue that was creating an empty file in the ./web folder --- web/scripts/build-locales.mjs | 23 ++++---- web/scripts/check-spelling.mjs | 1 + web/scripts/eslint-nightmare.mjs | 55 +++++++++++++++++++ web/scripts/patch-spotlight.sh | 2 +- ...k-application-wizard-commit-application.ts | 7 +-- .../PropertyMappingTestForm.ts | 15 ++--- web/src/common/helpers/webauthn.ts | 14 ++--- web/src/elements/Tabs.ts | 12 ++-- .../forms/SearchSelect/ak-search-select.ts | 11 ++-- web/src/elements/sidebar/SidebarItem.ts | 18 ++---- web/src/flow/stages/prompt/PromptStage.ts | 7 +-- .../details/UserSettingsFlowExecutor.ts | 5 +- .../details/stages/prompt/PromptStage.ts | 24 ++++---- 13 files changed, 114 insertions(+), 80 deletions(-) create mode 100644 web/scripts/eslint-nightmare.mjs diff --git a/web/scripts/build-locales.mjs b/web/scripts/build-locales.mjs index de8febbefa07..4cebd82fe98c 100644 --- a/web/scripts/build-locales.mjs +++ b/web/scripts/build-locales.mjs @@ -5,9 +5,9 @@ import process from "process"; const localizeRules = JSON.parse(fs.readFileSync("./lit-localize.json", "utf-8")); -function compareXlfAndSrc(loc) { - const xlf = path.join("./xliff", `${loc}.xlf`); - const src = path.join("./src/locales", `${loc}.ts`); +function generatedFileIsUpToDateWithXliffSource(loc) { + const xliff = path.join("./xliff", `${loc}.xlf`); + const gened = path.join("./src/locales", `${loc}.ts`); // Returns false if: the expected XLF file doesn't exist, The expected // generated file doesn't exist, or the XLF file is newer (has a higher date) @@ -15,29 +15,28 @@ function compareXlfAndSrc(loc) { // generates a unique error message and halts the build. try { - var xlfStat = fs.statSync(xlf); + var xlfStat = fs.statSync(xliff); } catch (_error) { console.error(`lit-localize expected '${loc}.xlf', but XLF file is not present`); process.exit(1); } + // If the generated file doesn't exist, of course it's not up to date. try { - var srcStat = fs.statSync(src); + var genedStat = fs.statSync(gened); } catch (_error) { return false; } - // if the xlf is newer (greater) than src, it's out of date. - if (xlfStat.mtimeMs > srcStat.mtimeMs) { - return false; - } - return true; + // if the generated file is the same age or older (date is greater) than the xliff file, it's + // presumed to have been generated by that file and is up-to-date. + return genedStat.mtimeMs >= xlfStat.mtimeMs; } // For all the expected files, find out if any aren't up-to-date. const upToDate = localizeRules.targetLocales.reduce( - (acc, loc) => acc && compareXlfAndSrc(loc), + (acc, loc) => acc && generatedFileIsUpToDateWithXliffSource(loc), true, ); @@ -61,7 +60,9 @@ if (!upToDate) { .map((locale) => `Locale '${locale}' has ${counts.get(locale)} missing translations`) .join("\n"); + // eslint-disable-next-line no-console console.log(`Translation tables rebuilt.\n${report}\n`); } +// eslint-disable-next-line no-console console.log("Locale ./src is up-to-date"); diff --git a/web/scripts/check-spelling.mjs b/web/scripts/check-spelling.mjs index 0633515956a8..2c419539a118 100644 --- a/web/scripts/check-spelling.mjs +++ b/web/scripts/check-spelling.mjs @@ -12,4 +12,5 @@ const cmd = [ "-S './src/locales/**' ./src -s", ].join(" "); +// eslint-disable-next-line no-console console.log(execSync(cmd, { encoding: "utf8" })); diff --git a/web/scripts/eslint-nightmare.mjs b/web/scripts/eslint-nightmare.mjs new file mode 100644 index 000000000000..0d463c21195c --- /dev/null +++ b/web/scripts/eslint-nightmare.mjs @@ -0,0 +1,55 @@ +import { execFileSync } from "child_process"; +import { ESLint } from "eslint"; +import path from "path"; +import process from "process"; + +// Code assumes this script is in the './web/scripts' folder. +const projectRoot = execFileSync("git", ["rev-parse", "--show-toplevel"], { + encoding: "utf8", +}).replace("\n", ""); +process.chdir(path.join(projectRoot, "./web")); + +const eslintConfig = { + overrideConfig: { + env: { + browser: true, + es2021: true, + }, + extends: [ + "eslint:recommended", + "plugin:@typescript-eslint/recommended", + "plugin:lit/recommended", + "plugin:custom-elements/recommended", + "plugin:storybook/recommended", + "plugin:sonarjs/recommended", + ], + parser: "@typescript-eslint/parser", + parserOptions: { + ecmaVersion: 12, + sourceType: "module", + }, + plugins: ["@typescript-eslint", "lit", "custom-elements", "sonarjs"], + rules: { + "indent": "off", + "linebreak-style": ["error", "unix"], + "quotes": ["error", "double", { avoidEscape: true }], + "semi": ["error", "always"], + "@typescript-eslint/ban-ts-comment": "off", + "sonarjs/cognitive-complexity": ["error", 9], + "sonarjs/no-duplicate-string": "off", + "sonarjs/no-nested-template-literals": "off", + }, + }, +}; + +const updated = ["./src/", "./build.mjs", "./scripts/*.mjs"]; + +const eslint = new ESLint(eslintConfig); +const results = await eslint.lintFiles(updated); +const formatter = await eslint.loadFormatter("stylish"); +const resultText = formatter.format(results); +const errors = results.reduce((acc, result) => acc + result.errorCount, 0); + +// eslint-disable-next-line no-console +console.log(resultText); +process.exit(errors > 1 ? 1 : 0); diff --git a/web/scripts/patch-spotlight.sh b/web/scripts/patch-spotlight.sh index fbb6e60402c9..a7886cf0ffb5 100644 --- a/web/scripts/patch-spotlight.sh +++ b/web/scripts/patch-spotlight.sh @@ -2,7 +2,7 @@ TARGET="./node_modules/@spotlightjs/overlay/dist/index-"[0-9a-f]*.js -if [[ $(grep -L "QX2" "$TARGET" > /dev/null 2&>1) ]]; then +if [[ $(grep -L "QX2" "$TARGET" > /dev/null 2> /dev/null) ]]; then patch --forward -V none --no-backup-if-mismatch -p0 $TARGET < { `; } - renderExampleButtons(): TemplateResult { - const header = html`

${msg("Example context data")}

`; - switch (this.mapping?.metaModelName) { - case "authentik_sources_ldap.ldappropertymapping": - return html`${header}${this.renderExampleLDAP()}`; - default: - return html``; - } + renderExampleButtons() { + return this.mapping?.metaModelName === "authentik_sources_ldap.ldappropertymapping" + ? html`

${msg("Example context data")}

+ ${this.renderExampleLDAP()}` + : nothing; } renderExampleLDAP(): TemplateResult { diff --git a/web/src/common/helpers/webauthn.ts b/web/src/common/helpers/webauthn.ts index 13a9894503e5..68b29ce76064 100644 --- a/web/src/common/helpers/webauthn.ts +++ b/web/src/common/helpers/webauthn.ts @@ -42,12 +42,11 @@ export function transformCredentialCreateOptions( user.id = u8arr(b64enc(u8arr(stringId))); const challenge = u8arr(credentialCreateOptions.challenge.toString()); - const transformedCredentialCreateOptions = Object.assign({}, credentialCreateOptions, { + return { + ...credentialCreateOptions, challenge, user, - }); - - return transformedCredentialCreateOptions; + }; } export interface Assertion { @@ -98,12 +97,11 @@ export function transformCredentialRequestOptions( }, ); - const transformedCredentialRequestOptions = Object.assign({}, credentialRequestOptions, { + return { + ...credentialRequestOptions, challenge, allowCredentials, - }); - - return transformedCredentialRequestOptions; + }; } export interface AuthAssertion { diff --git a/web/src/elements/Tabs.ts b/web/src/elements/Tabs.ts index cf3d4724da4a..55455bd4238f 100644 --- a/web/src/elements/Tabs.ts +++ b/web/src/elements/Tabs.ts @@ -92,11 +92,13 @@ export class Tabs extends AKElement { const pages = Array.from(this.querySelectorAll(":scope > [slot^='page-']")); if (window.location.hash.includes(ROUTE_SEPARATOR)) { const params = getURLParams(); - if (this.pageIdentifier in params && !this.currentPage) { - if (this.querySelector(`[slot='${params[this.pageIdentifier]}']`) !== null) { - // To update the URL to match with the current slot - this.onClick(params[this.pageIdentifier] as string); - } + if ( + this.pageIdentifier in params && + !this.currentPage && + this.querySelector(`[slot='${params[this.pageIdentifier]}']`) !== null + ) { + // To update the URL to match with the current slot + this.onClick(params[this.pageIdentifier] as string); } } if (!this.currentPage) { diff --git a/web/src/elements/forms/SearchSelect/ak-search-select.ts b/web/src/elements/forms/SearchSelect/ak-search-select.ts index cf3433112f35..d5e316028312 100644 --- a/web/src/elements/forms/SearchSelect/ak-search-select.ts +++ b/web/src/elements/forms/SearchSelect/ak-search-select.ts @@ -352,10 +352,13 @@ export class SearchSelect extends CustomEmitterElement(AKElement) { const onFocus = (ev: FocusEvent) => { this.open = true; this.renderMenu(); - if (this.blankable && this.renderedValue === this.emptyOption) { - if (ev.target && ev.target instanceof HTMLInputElement) { - ev.target.value = ""; - } + if ( + this.blankable && + this.renderedValue === this.emptyOption && + ev.target && + ev.target instanceof HTMLInputElement + ) { + ev.target.value = ""; } }; diff --git a/web/src/elements/sidebar/SidebarItem.ts b/web/src/elements/sidebar/SidebarItem.ts index 93f7e7b990ba..3cd6ca89e68a 100644 --- a/web/src/elements/sidebar/SidebarItem.ts +++ b/web/src/elements/sidebar/SidebarItem.ts @@ -117,19 +117,11 @@ export class SidebarItem extends AKElement { if (!this.path) { return false; } - if (this.path) { - const ourPath = this.path.split(";")[0]; - if (new RegExp(`^${ourPath}$`).exec(path)) { - return true; - } - } - return this.activeMatchers.some((v) => { - const match = v.exec(path); - if (match !== null) { - return true; - } - return false; - }); + + const ourPath = this.path.split(";")[0]; + const pathIsWholePath = new RegExp(`^${ourPath}$`).test(path); + const pathIsAnActivePath = this.activeMatchers.some((v) => v.test(path)); + return pathIsWholePath || pathIsAnActivePath; } expandParentRecursive(activePath: string, item: SidebarItem): void { diff --git a/web/src/flow/stages/prompt/PromptStage.ts b/web/src/flow/stages/prompt/PromptStage.ts index 5c8b8d588f69..9453dac3e64d 100644 --- a/web/src/flow/stages/prompt/PromptStage.ts +++ b/web/src/flow/stages/prompt/PromptStage.ts @@ -231,14 +231,11 @@ ${prompt.initialValue} { this.challenge = data; - if (this.challenge.responseErrors) { - return false; - } - return true; + return !this.challenge.responseErrors; }) .catch((e: Error | ResponseError) => { this.errorMessage(e); diff --git a/web/src/user/user-settings/details/stages/prompt/PromptStage.ts b/web/src/user/user-settings/details/stages/prompt/PromptStage.ts index 6f9fefdb9099..377b4d5c1556 100644 --- a/web/src/user/user-settings/details/stages/prompt/PromptStage.ts +++ b/web/src/user/user-settings/details/stages/prompt/PromptStage.ts @@ -10,20 +10,16 @@ import { PromptTypeEnum, StagePrompt } from "@goauthentik/api"; @customElement("ak-user-stage-prompt") export class UserSettingsPromptStage extends PromptStage { renderPromptInner(prompt: StagePrompt): TemplateResult { - switch (prompt.type) { - // Checkbox requires slightly different rendering here due to the use of horizontal form elements - case PromptTypeEnum.Checkbox: - return html``; - default: - return super.renderPromptInner(prompt); - } + return prompt.type === PromptTypeEnum.Checkbox + ? html`` + : super.renderPromptInner(prompt); } renderField(prompt: StagePrompt): TemplateResult {