diff --git a/frontend/DeveloperGuide.md b/frontend/DeveloperGuide.md index f0ec58cbec..ab4d04a617 100644 --- a/frontend/DeveloperGuide.md +++ b/frontend/DeveloperGuide.md @@ -45,6 +45,84 @@ export function f2() { /* ... */ } // Custom desktop implementation Building with this pattern minimises the duplicated code in the system and greatly reduces the chances that changing the core app will break the desktop app. +### Naming extension modules + +Extension modules and the functions/hooks they export should be named after **what they do**, not **which build overrides them**. +Core code must never reference build targets (desktop, saas, etc.) by name — it should simply call a generic extension point and remain unaware of which layer is providing the implementation. + +```ts +// ✅ CORRECT - named after the behaviour, not the build +// core/useFrontendVersionInfo.ts +export function useFrontendVersionInfo() { /* stub */ } + +// desktop/useFrontendVersionInfo.ts +export function useFrontendVersionInfo() { /* real Tauri implementation */ } +``` + +```ts +// ❌ WRONG - core code reveals knowledge of the desktop layer +// core/useDesktopVersionInfo.ts +export function useDesktopVersionInfo() { /* stub */ } +``` + +Similarly, core code should never contain conditionals that check which build is active (e.g. `if (isDesktop)`). +If behaviour needs to vary, that variation belongs in an extension module - the core simply calls it. + +The same principle applies in reverse: code inside `desktop/` is guaranteed to be running in the Tauri environment, so `isTauri()` checks are never needed there either. +If you find yourself writing `if (isDesktop())` or `if (isTauri())` anywhere, that is a sign the extension point has not been modelled correctly - the build system is already doing that separation for you. + +### List extensions + +When a build needs to _add_ behaviour rather than _replace_ it, the extension module can return a list of items and let core manage the rendering. +Core defines the function to return an empty list; the extension build overrides it to return a populated one. + +```ts +// core/toolbarExtensions.ts +export interface ToolbarButton { + label: string; + onClick: () => void; +} + +export function getToolbarButtons(): ToolbarButton[] { + return []; +} +``` + +```ts +// desktop/toolbarExtensions.ts +import { type ToolbarButton } from '@core/toolbarExtensions'; +export { type ToolbarButton }; + +export function getToolbarButtons(): ToolbarButton[] { + return [ + { label: 'Open folder', onClick: () => { /* ... */ } }, + ]; +} +``` + +```tsx +// core/Toolbar.tsx +import { getToolbarButtons } from '@app/toolbarExtensions'; + +export function Toolbar() { + return ( +
+ + + {getToolbarButtons().map((button) => ( + + ))} +
+ ); +} +``` + +This pattern works well for things like menu items or toolbar actions - anything where a build contributes additional entries to a well-defined set. + +### Import aliases + In general, all imports for app code should come via `@app` because it allows for other builds of the app to override behaviour if necessary. The only time that it is beneficial to import via a specific folder (e.g. `@core`) is when you want to reduce duplication **in the file you are overriding**. For example: diff --git a/frontend/eslint.config.mjs b/frontend/eslint.config.mjs index b8cbc95a1d..6f0ed90cbb 100644 --- a/frontend/eslint.config.mjs +++ b/frontend/eslint.config.mjs @@ -13,6 +13,11 @@ const nodeGlobs = [ '*.config.{js,ts,mjs}', ]; +const baseRestrictedImportPatterns = [ + { regex: '^\\.', message: "Use @app/* imports instead of relative imports." }, + { regex: '^src/', message: "Use @app/* imports instead of absolute src/ imports." }, +]; + export default defineConfig( { // Everything that contains 3rd party code that we don't want to lint @@ -30,10 +35,7 @@ export default defineConfig( 'no-restricted-imports': [ 'error', { - patterns: [ - ".*", // Disallow any relative imports (they should be '@app/x/y/z' or similar) - "src/*", // Disallow any absolute imports (they should be '@app/x/y/z' or similar) - ], + patterns: baseRestrictedImportPatterns, }, ], '@typescript-eslint/no-empty-object-type': [ @@ -59,6 +61,26 @@ export default defineConfig( ], }, }, + // Desktop-only packages must not be imported from core or proprietary code. + // Use the stub/shadow pattern instead: define a stub in src/core/ and override in src/desktop/. + { + files: srcGlobs, + ignores: ['src/desktop/**'], + rules: { + 'no-restricted-imports': [ + 'error', + { + patterns: [ + ...baseRestrictedImportPatterns, + { + regex: '^@tauri-apps/', + message: "Tauri APIs are desktop-only. Review frontend/DeveloperGuide.md for structure advice.", + }, + ], + }, + ], + }, + }, // Folders that have been cleaned up and are now conformant - stricter rules enforced here { files: ['src/saas/**/*.{js,mjs,jsx,ts,tsx}'], diff --git a/frontend/src/core/components/shared/config/configSections/GeneralSection.tsx b/frontend/src/core/components/shared/config/configSections/GeneralSection.tsx index db3eb107f7..2d4c9f42de 100644 --- a/frontend/src/core/components/shared/config/configSections/GeneralSection.tsx +++ b/frontend/src/core/components/shared/config/configSections/GeneralSection.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useMemo } from "react"; +import React, { useState, useEffect } from "react"; import { Paper, Stack, @@ -22,8 +22,7 @@ import type { ToolPanelMode } from "@app/constants/toolPanel"; import LocalIcon from "@app/components/shared/LocalIcon"; import { updateService, UpdateSummary } from "@app/services/updateService"; import UpdateModal from "@app/components/shared/UpdateModal"; -import { getVersion } from "@tauri-apps/api/app"; -import { isTauri } from "@tauri-apps/api/core"; +import { useFrontendVersionInfo } from "@app/hooks/useFrontendVersionInfo"; const DEFAULT_AUTO_UNZIP_FILE_LIMIT = 4; const BANNER_DISMISSED_KEY = "stirlingpdf_features_banner_dismissed"; @@ -46,10 +45,8 @@ const GeneralSection: React.FC = ({ hideTitle = false, hide const [updateSummary, setUpdateSummary] = useState(null); const [updateModalOpened, setUpdateModalOpened] = useState(false); const [checkingUpdate, setCheckingUpdate] = useState(false); - const [mismatchVersion, setMismatchVersion] = useState(false); - const isTauriApp = useMemo(() => isTauri(), []); - const [appVersion, setAppVersion] = useState(null); - const frontendVersionLabel = appVersion ?? t("common.loading", "Loading..."); + const { appVersion, mismatchVersion } = useFrontendVersionInfo(config?.appVersion); + const frontendVersionLabel = appVersion ?? t("common.loading", "Loading..."); // null = loading, shown only when appVersion !== undefined // Sync local state with preference changes useEffect(() => { @@ -91,52 +88,6 @@ const GeneralSection: React.FC = ({ hideTitle = false, hide setCheckingUpdate(false); }; - useEffect(() => { - if (!isTauriApp) { - setMismatchVersion(false); - return; - } - - let cancelled = false; - const fetchFrontendVersion = async () => { - try { - const frontendVersion = await getVersion(); - if (!cancelled) { - setAppVersion(frontendVersion); - } - } catch (error) { - console.error("[GeneralSection] Failed to fetch frontend version:", error); - } - }; - - fetchFrontendVersion(); - - return () => { - cancelled = true; - }; - }, [isTauriApp]); - - useEffect(() => { - if (!isTauriApp) { - return; - } - - if (!appVersion || !config?.appVersion) { - setMismatchVersion(false); - return; - } - - if (appVersion !== config.appVersion) { - console.warn("[GeneralSection] Mismatch between Tauri version and AppConfig version:", { - backendVersion: config.appVersion, - frontendVersion: appVersion, - }); - setMismatchVersion(true); - } else { - setMismatchVersion(false); - } - }, [isTauriApp, appVersion, config?.appVersion]); - // Check if login is disabled const loginDisabled = !config?.enableLogin; @@ -239,7 +190,7 @@ const GeneralSection: React.FC = ({ hideTitle = false, hide )} - {isTauriApp && ( + {appVersion !== undefined && (
diff --git a/frontend/src/core/hooks/useFrontendVersionInfo.ts b/frontend/src/core/hooks/useFrontendVersionInfo.ts new file mode 100644 index 0000000000..98dede73cc --- /dev/null +++ b/frontend/src/core/hooks/useFrontendVersionInfo.ts @@ -0,0 +1,8 @@ +export interface FrontendVersionInfo { + appVersion: string | null | undefined; // undefined = not applicable, null = loading, string = loaded + mismatchVersion: boolean; +} + +export function useFrontendVersionInfo(_backendVersion: string | undefined): FrontendVersionInfo { + return { appVersion: undefined, mismatchVersion: false }; +} diff --git a/frontend/src/desktop/hooks/useFrontendVersionInfo.ts b/frontend/src/desktop/hooks/useFrontendVersionInfo.ts new file mode 100644 index 0000000000..1bc0b60440 --- /dev/null +++ b/frontend/src/desktop/hooks/useFrontendVersionInfo.ts @@ -0,0 +1,44 @@ +import { useState, useEffect } from "react"; +import { getVersion } from "@tauri-apps/api/app"; +import type { FrontendVersionInfo } from "@core/hooks/useFrontendVersionInfo"; + +export function useFrontendVersionInfo(backendVersion: string | undefined): FrontendVersionInfo { + const [appVersion, setAppVersion] = useState(null); + const [mismatchVersion, setMismatchVersion] = useState(false); + + useEffect(() => { + let cancelled = false; + const fetchVersion = async () => { + try { + const version = await getVersion(); + if (!cancelled) { + setAppVersion(version); + } + } catch (error) { + console.error("[useFrontendVersionInfo] Failed to fetch frontend version:", error); + } + }; + fetchVersion(); + return () => { + cancelled = true; + }; + }, []); + + useEffect(() => { + if (!appVersion || !backendVersion) { + setMismatchVersion(false); + return; + } + if (appVersion !== backendVersion) { + console.warn("[useFrontendVersionInfo] Mismatch between frontend version and AppConfig version:", { + backendVersion, + frontendVersion: appVersion, + }); + setMismatchVersion(true); + } else { + setMismatchVersion(false); + } + }, [appVersion, backendVersion]); + + return { appVersion, mismatchVersion }; +}