diff --git a/Makefile b/Makefile index 74e51886b..af1fe3188 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build clean ui +.PHONY: build clean ui check-ui check-ui-assets check-ui-locales check-ui-plugin-i18n VERSION=2.0.3 BIN=answer @@ -47,6 +47,22 @@ check: test: @$(GO) test ./internal/repo/repo_test +# Frontend checks for behaviour a successful build does not demonstrate. +# Each guards a runtime failure that leaves every build step reporting success. +check-ui: check-ui-assets check-ui-locales check-ui-plugin-i18n + +# The server reads the built asset paths out of index.html. +check-ui-assets: + @./script/check-built-assets.sh + +# The app loads languages other than the default one through a dynamic import. +check-ui-locales: + @cd ui && pnpm check-locales + +# Plugin translations register while modules evaluate, in bundler-decided order. +check-ui-plugin-i18n: + @cd ui && pnpm check-plugin-i18n + # clean all build result clean: @$(GO) clean ./... diff --git a/script/check-built-assets.sh b/script/check-built-assets.sh new file mode 100755 index 000000000..6e8d2cc8b --- /dev/null +++ b/script/check-built-assets.sh @@ -0,0 +1,104 @@ +#!/bin/bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you 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. + +# Builds the frontend and asserts the server can still find the built assets +# inside index.html. See internal/controller/template_controller_test.go for +# why that is not implied by a successful build. +# +# --skip-build reuse an existing ui/build, do not rebuild +# --self-check additionally rewrite ui/build/index.html so a script +# or stylesheet tag is missing, and confirm the check +# fails on the missing asset. Restores the real build +# output afterwards. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +INDEX_HTML="$REPO_ROOT/ui/build/index.html" +TEST_PACKAGE="./internal/controller/" +TEST_NAME="TestGetStyleResolvesBuiltAssets" + +list_output="$(cd "$REPO_ROOT" && go test "$TEST_PACKAGE" -list "^${TEST_NAME}$" 2>&1)" +if ! grep -qx "$TEST_NAME" <<<"$list_output"; then + echo "no test named $TEST_NAME in $TEST_PACKAGE; go test -run with a stale/renamed test name matches nothing and still exits 0, which would turn this check into a silent no-op" >&2 + exit 1 +fi + +skip_build=0 +self_check=0 +for arg in "$@"; do + case "$arg" in + --skip-build) skip_build=1 ;; + --self-check) self_check=1 ;; + *) echo "unknown option: $arg" >&2; exit 2 ;; + esac +done + +run_check() { + (cd "$REPO_ROOT" && go test -count=1 "$TEST_PACKAGE" -run "$TEST_NAME" "$@") +} + +if [ "$skip_build" -eq 0 ]; then + echo "==> building frontend" + (cd "$REPO_ROOT/ui" && pnpm build) +fi + +if [ ! -f "$INDEX_HTML" ]; then + echo "no built index.html at $INDEX_HTML; run without --skip-build" >&2 + exit 1 +fi + +echo "==> checking the server can parse the built asset tags" +run_check -v + +if [ "$self_check" -eq 0 ]; then + exit 0 +fi + +# Confirm the check actually fails when a required asset is missing from +# the build output. Without this, a check that silently stopped asserting +# anything would look identical to a passing one. +backup="$(mktemp)" +cp "$INDEX_HTML" "$backup" +trap 'cp "$backup" "$INDEX_HTML"; rm -f "$backup"' EXIT + +expect_failure() { + local label="$1" + local html="$2" + printf '%s' "$html" > "$INDEX_HTML" + echo "==> self-check: expecting failure on $label" + if run_check >/dev/null 2>&1; then + echo "SELF-CHECK FAILED: the check passed on $label, so it is not guarding anything" >&2 + exit 1 + fi + echo " check failed as expected" +} + +expect_failure "stylesheet link but no script src" \ + '
' + +expect_failure "script src but no stylesheet link" \ + '
' + +expect_failure "only an inline script, no src" \ + '
' + +expect_failure "manifest link but no stylesheet link" \ + '
' + +echo "==> self-check passed" diff --git a/ui/package.json b/ui/package.json index 07e5293f3..9a252a6b9 100644 --- a/ui/package.json +++ b/ui/package.json @@ -12,6 +12,8 @@ "build:packages": "pnpm -r --filter=./src/plugins/* run build", "clean": "rm -rf node_modules && rm -rf src/plugins/**/node_modules", "analyze": "source-map-explorer 'build/static/js/*.js'", + "check-locales": "node ./scripts/check-locale-resolution.js", + "check-plugin-i18n": "node ./scripts/check-plugin-i18n-order.js", "setup-lint": "node scripts/setup-eslint.js && cd .. && husky install", "lint": "eslint . --cache --fix --ext .ts,.tsx", "prettier": "prettier --write \"src/**/*.{ts,tsx,css,scss,md}\"", diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js new file mode 100644 index 000000000..abdc89b09 --- /dev/null +++ b/ui/scripts/check-locale-resolution.js @@ -0,0 +1,281 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +/* + * src/utils/localize.ts loads a language with a template-literal dynamic + * import through the @i18n alias, which points outside the frontend root. A + * build tool that cannot resolve that shape still produces a clean build and + * still serves a working app; the resources simply never arrive. The failure + * is confined to runtime and to languages other than the default one, so + * neither a green build nor a smoke test in the default language sees it. + * + * So: start the project's own dev server, load that same import through it, + * and require two different languages to come back as different, real + * translated content. + * + * Two separate things have to hold, and only one of them is about resolution: + * + * 1. The module graph resolves the alias and parses the file. Covered by + * importing the probe through the server's module runner. + * 2. A browser is actually allowed to fetch it. The languages live outside + * the frontend root, so they are reachable only if the dev server's + * filesystem allow-list includes their directory. The module runner does + * not go through that allow-list. A real request does, so this makes one. + * + * Checking only the first would pass while every language 403s in a browser. + */ + +const fs = require('fs'); +const path = require('path'); +const util = require('util'); + +const UI_DIR = path.resolve(__dirname, '..'); +const PROBE = path.resolve(__dirname, 'locale-probe.js'); +const LOCALIZE_SOURCE = path.resolve(UI_DIR, 'src/utils/localize.ts'); +const DEV_SERVER_CONFIG = path.resolve(UI_DIR, 'vite.config.mts'); + +// A language written in a script the default language does not use, so +// "resolved the language that was asked for" cannot be mistaken for "fell +// back to the default language". +const TARGET_LANG = 'zh_CN'; +const DEFAULT_LANG = 'en_US'; +const TARGET_SCRIPT = /[一-鿿]/; + +// The check is a failure until it proves otherwise, so an unexpected +// event-loop drain (a pending promise with no live handles) cannot exit 0. +process.exitCode = 1; + +// unref lets a finished run exit on time; a run held alive by any leaked +// or hung handle still gets terminated with a verdict. +// Must exceed the sum of every bounded step below (worst case 100s), or a +// run whose steps all succeed slowly gets a false FAIL from its own backstop. +const WATCHDOG_MS = 180000; +const watchdog = setTimeout(() => { + console.error(`FAIL: check did not complete within ${WATCHDOG_MS / 1000}s`); + process.exit(1); +}, WATCHDOG_MS); +watchdog.unref(); + +const rel = (file) => path.relative(UI_DIR, file); + +function fail(message) { + throw new Error(`FAIL: ${message}`); +} + +function withTimeout(promise, ms, message) { + let timer; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`FAIL: ${message}`)), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); +} + +// The probe carries a copy of the application's import expression, so it is +// only evidence for as long as the two agree. +function assertProbeStillMatchesApplication() { + const source = fs.readFileSync(LOCALIZE_SOURCE, 'utf8'); + if (!/import\(\s*`@i18n\/\$\{[^}]+\}\.yaml`\s*\)/.test(source)) { + fail( + `${rel(LOCALIZE_SOURCE)} no longer loads languages with a template-literal import ` + + `through @i18n, so ${rel(PROBE)} is exercising a shape the application does not use. ` + + `Update the probe to match the application, then re-run.`, + ); + } +} + +async function openProbe() { + if (!fs.existsSync(DEV_SERVER_CONFIG)) { + fail( + `no dev server configuration this check knows how to drive was found at ` + + `${rel(DEV_SERVER_CONFIG)}; teach it how to load ${rel(PROBE)} with the current ` + + `tooling before relying on it again`, + ); + } + + const { createServer, createServerModuleRunner } = await import('vite'); + const server = await withTimeout( + createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + }), + 30000, + 'dev server creation did not complete within 30s', + ); + try { + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); + } catch (err) { + // listen() failed or timed out, but the server (and the watcher and + // websocket server it created before listen() ever ran) already exists. + // Nothing else will hold a reference to it once this throws, so close it + // here or it keeps the event loop alive forever. + await withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}); + throw err; + } + + const ssr = server.environments.ssr; + const baseUrl = (server.resolvedUrls.local[0] || '').replace(/\/$/, ''); + + // Loaded on demand rather than up front, because the reachability check + // below is only meaningful before anything pulls a language into the module + // graph. See the comment on assertBrowserCanFetch. + let probe = null; + const importProbe = async () => { + if (!probe) { + probe = await withTimeout( + createServerModuleRunner(ssr).import(PROBE), + 30000, + 'module runner did not import the probe within 30s', + ); + } + return probe; + }; + + return { + load: async (langName) => + (await importProbe()).loadLocaleResource(langName), + + // Ask for the language file the way the browser will: over HTTP, at the + // path the dev server assigns to a file outside the frontend root. + // + // ORDER MATTERS. Run this before any language is loaded through the module + // graph. Once a module is in the graph the dev server answers from the + // transform pipeline instead of reading the file, and the request stops + // passing through the filesystem allow-list. Checking afterwards returns + // 200 even when the allow-list would give a browser a 403, which is to say + // it checks nothing. Resolve the path without loading it, then ask. + async assertBrowserCanFetch(langName) { + const specifier = `@i18n/${langName}.yaml`; + const resolved = await ssr.pluginContainer.resolveId(specifier, PROBE); + + if (!resolved || !resolved.id) { + fail( + `${specifier} does not resolve at all, so there is nothing for a browser to request`, + ); + } + + const url = `${baseUrl}/@fs${resolved.id.split('?')[0]}`; + let response; + try { + response = await fetch(url); + } catch (err) { + fail( + `requesting ${langName} at ${url} failed outright: ${err.message}`, + ); + } + + if (!response.ok) { + fail( + `the dev server answered ${response.status} for ${langName} at ${url}; the language ` + + `files sit outside the frontend root, so a browser would get this too and every ` + + `language other than the default would fail to load`, + ); + } + + const body = await response.text(); + if (!TARGET_SCRIPT.test(body)) { + fail( + `the dev server served ${langName} at ${url} but the response carries none of its ` + + `script; a browser would receive something that is not the translated file`, + ); + } + }, + + close: () => + withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}), + }; +} + +function resourcesOf(resConf, langName) { + // The application reads .ui off the loaded file and registers it with i18next. + const resources = resConf && resConf.ui; + if ( + !resources || + typeof resources !== 'object' || + Object.keys(resources).length === 0 + ) { + fail( + `${langName} resolved to ${util.inspect(resConf, { depth: 1 })}, which carries no ui ` + + `section; the application would register no translations for it`, + ); + } + return resources; +} + +async function main() { + assertProbeStillMatchesApplication(); + + const probe = await openProbe(); + try { + // Before any load, while the filesystem allow-list still governs the request. + await probe.assertBrowserCanFetch(TARGET_LANG); + + const target = resourcesOf(await probe.load(TARGET_LANG), TARGET_LANG); + const fallback = resourcesOf(await probe.load(DEFAULT_LANG), DEFAULT_LANG); + + if (JSON.stringify(target) === JSON.stringify(fallback)) { + fail( + `${TARGET_LANG} and ${DEFAULT_LANG} resolved to identical resources; the language ` + + `name is not selecting a file, so every language would render as ${DEFAULT_LANG}`, + ); + } + + if (!TARGET_SCRIPT.test(JSON.stringify(target))) { + fail( + `${TARGET_LANG} resolved without a single character of its own script; the content ` + + `is not the translated file`, + ); + } + + console.log( + `OK: ${TARGET_LANG} and ${DEFAULT_LANG} resolve at runtime to distinct translated ` + + `resources, and ${TARGET_LANG} is fetchable over the dev server`, + ); + } finally { + await withTimeout( + probe.close(), + 10000, + 'probe close did not complete within 10s', + ).catch(() => {}); + } +} + +// Every finally above has already run by the time either callback below +// fires, so a hard exit here cannot skip cleanup; it only guarantees +// termination on both outcomes, including when a bounded-but-hung close +// would otherwise keep a finished check alive after success. +main().then( + () => process.exit(0), + (err) => { + console.error(err.message || err.stack || String(err)); + process.exit(1); + }, +); diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js new file mode 100644 index 000000000..5c87895ec --- /dev/null +++ b/ui/scripts/check-plugin-i18n-order.js @@ -0,0 +1,236 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +/* + * Plugin i18n modules call initI18nResource while they are being evaluated, + * and i18next only attaches its resource-store methods to the instance while + * init runs. So whether registering a plugin's translations works depends on + * whether that module evaluated before or after init, which is decided by how + * the bundler groups and orders chunks. Nothing in the application controls + * it, and when it goes wrong the entry module throws while still evaluating: + * the build succeeds, the server returns a page, the console stays empty, and + * the application never mounts. + * + * Registering BEFORE init is the order that breaks, so do exactly that. Then + * initialise, then require the translations to actually be present. That last + * step is the point: a fix that merely swallowed the error would leave every + * plugin string untranslated and would otherwise look identical to a working + * one. + */ + +const fs = require('fs'); +const path = require('path'); +const util = require('util'); + +const UI_DIR = path.resolve(__dirname, '..'); +const PLUGIN_UTILS = path.resolve(UI_DIR, 'src/utils/pluginKit/utils.ts'); +const DEV_SERVER_CONFIG = path.resolve(UI_DIR, 'vite.config.mts'); + +const LANG = 'en_US'; +const PLUGIN_NS = 'plugin'; +const SLUG = 'check_only_plugin'; +const SENTINEL = 'registered before init'; + +// The check is a failure until it proves otherwise, so an unexpected +// event-loop drain (a pending promise with no live handles) cannot exit 0. +process.exitCode = 1; + +// unref lets a finished run exit on time; a run held alive by any leaked +// or hung handle still gets terminated with a verdict. +// Must exceed the sum of every bounded step below (worst case 130s), or a +// run whose steps all succeed slowly gets a false FAIL from its own backstop. +const WATCHDOG_MS = 180000; +const watchdog = setTimeout(() => { + console.error(`FAIL: check did not complete within ${WATCHDOG_MS / 1000}s`); + process.exit(1); +}, WATCHDOG_MS); +watchdog.unref(); + +const rel = (file) => path.relative(UI_DIR, file); + +function fail(message) { + throw new Error(`FAIL: ${message}`); +} + +function withTimeout(promise, ms, message) { + let timer; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`FAIL: ${message}`)), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); +} + +async function main() { + if (!fs.existsSync(DEV_SERVER_CONFIG)) { + fail( + `no dev server configuration this check knows how to drive was found at ` + + `${rel(DEV_SERVER_CONFIG)}; teach it how to load ${rel(PLUGIN_UTILS)} with the ` + + `current tooling before relying on it again`, + ); + } + + const { createServer, createServerModuleRunner } = await import('vite'); + const server = await withTimeout( + createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + // Without this the helper under test and this check each resolve + // their own copy of i18next, and the check ends up inspecting an + // instance nobody registered anything into. It reads as a failure + // with a confusing message rather than as a broken harness, so the + // single-instance assertion below guards it too. + ssr: { noExternal: ['i18next'] }, + }), + 30000, + 'dev server creation did not complete within 30s', + ); + try { + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); + + const runner = createServerModuleRunner(server.environments.ssr); + + // Deliberately load the plugin helper first and never touch the app's own + // i18n bootstrap, so i18next is guaranteed to be uninitialised here. This + // is the ordering the bundler is free to produce. + const pluginUtils = await withTimeout( + runner.import(PLUGIN_UTILS), + 30000, + 'module runner did not import the plugin utils within 30s', + ); + const i18nextModule = await withTimeout( + runner.import('i18next'), + 30000, + 'module runner did not import i18next within 30s', + ); + const i18next = i18nextModule.default; + + // If the helper and this check hold different copies, every assertion + // below is measuring an object nothing under test ever touched. + const loaded = [ + ...server.environments.ssr.moduleGraph.idToModuleMap.keys(), + ].filter( + (id) => /[\\/]i18next[\\/]/.test(id) && !/[\\/]\.vite[\\/]/.test(id), + ); + if (loaded.length !== 1) { + fail( + `expected exactly one i18next module to be loaded, found ${loaded.length}: ` + + `${util.inspect(loaded)}. This check can only observe what the helper registers ` + + `if both resolve the same copy.`, + ); + } + + if (i18next.isInitialized) { + fail( + `i18next was already initialised before this check registered anything, so the ` + + `ordering the check exists to exercise was never exercised; the check is not ` + + `proving what it claims`, + ); + } + + const resource = { + [LANG]: { plugin: { [SLUG]: { ui: { title: SENTINEL } } } }, + }; + + try { + pluginUtils.initI18nResource(resource); + } catch (err) { + fail( + `registering a plugin's translations before i18next.init threw: ${err.message}\n` + + ` This is the ordering a bundler is free to produce. When it happens the entry ` + + `module throws while evaluating and the application never mounts, with no console ` + + `output and a successful build.`, + ); + } + + await i18next.init({ lng: LANG, fallbackLng: LANG, resources: {} }); + + const bundle = i18next.getResourceBundle(LANG, PLUGIN_NS); + const title = + bundle && bundle[SLUG] && bundle[SLUG].ui && bundle[SLUG].ui.title; + + if (title !== SENTINEL) { + fail( + `registering before init did not throw, but the translations never arrived: ` + + `expected ${util.inspect(SENTINEL)} at ${PLUGIN_NS}.${SLUG}.ui.title, got ` + + `${util.inspect(bundle)}.\n` + + ` Surviving the call is not enough. Plugin strings have to actually be ` + + `registered once i18next is up, or every plugin renders untranslated.`, + ); + } + + // i18next.init already ran above, and the 'initialized' event it fires + // has already been consumed by the first scenario's listener. Registering + // a second plugin now only works if initI18nResource also takes the + // immediate branch, which had no coverage until this scenario existed. + const slugAfterInit = 'check_only_plugin_after_init'; + const sentinelAfterInit = 'registered after init'; + + pluginUtils.initI18nResource({ + [LANG]: { + plugin: { [slugAfterInit]: { ui: { title: sentinelAfterInit } } }, + }, + }); + const bundleAfterInit = i18next.getResourceBundle(LANG, PLUGIN_NS); + const titleAfterInit = + bundleAfterInit && + bundleAfterInit[slugAfterInit] && + bundleAfterInit[slugAfterInit].ui && + bundleAfterInit[slugAfterInit].ui.title; + + if (titleAfterInit !== sentinelAfterInit) { + fail( + `registering a plugin's translations after i18next.init did not land ` + + `synchronously: expected ${util.inspect(sentinelAfterInit)} at ` + + `${PLUGIN_NS}.${slugAfterInit}.ui.title, got ${util.inspect(bundleAfterInit)}.\n` + + ` The 'initialized' event already fired for the first scenario, so this ` + + `only passes if initI18nResource also registers immediately when i18next ` + + `is already initialised. Missing that branch means every plugin loaded ` + + `after i18next.init renders untranslated.`, + ); + } + + console.log( + `OK: plugin translations registered before i18next.init (deferred) and ` + + `after i18next.init (immediate) both survive and are present`, + ); + } finally { + await withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}); + } +} + +// Every finally above has already run by the time either callback below +// fires, so a hard exit here cannot skip cleanup; it only guarantees +// termination on both outcomes, including when a bounded-but-hung close +// would otherwise keep a finished check alive after success. +main().then( + () => process.exit(0), + (err) => { + console.error(err.message || err.stack || String(err)); + process.exit(1); + }, +); diff --git a/ui/scripts/locale-probe.js b/ui/scripts/locale-probe.js new file mode 100644 index 000000000..ac5eadf72 --- /dev/null +++ b/ui/scripts/locale-probe.js @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +/* + * Mirrors how src/utils/localize.ts loads a language at runtime. The import + * specifier is a template literal resolved through an alias that points + * outside the frontend root, so whether it resolves is a property of the + * bundler rather than of this file. + * + * Keep this expression identical to the one in the application. + * check-locale-resolution.js asserts that it still matches. + */ +export const loadLocaleResource = async (langName) => { + const { default: resConf } = await import(`@i18n/${langName}.yaml`); + return resConf; +}; diff --git a/ui/src/components/Comment/index.scss b/ui/src/components/Comment/index.scss index 7264c66c4..415fd25e9 100644 --- a/ui/src/components/Comment/index.scss +++ b/ui/src/components/Comment/index.scss @@ -23,6 +23,7 @@ .comments-wrap { .comment-item { + border-bottom: 1px solid var(--an-comment-item-border-bottom); &:hover { @include media-breakpoint-up(md) { .control-area { @@ -30,7 +31,6 @@ } } } - border-bottom: 1px solid var(--an-comment-item-border-bottom); } .fmt { display: inline; diff --git a/ui/src/router/routes.ts b/ui/src/router/routes.ts index 8423fb7a3..e0b48c809 100644 --- a/ui/src/router/routes.ts +++ b/ui/src/router/routes.ts @@ -537,7 +537,7 @@ const routes: RouteNode[] = [ }, { path: '403', - page: 'pages/403', + page: 'pages/404/403', }, ], },