refactor: use interfaces where possible instead of instances

__tests__/navigation.spec.ts

@@ -4,9 +4,7 @@
  */
 import { describe, it, expect, vi } from 'vitest'
 import { Navigation, getNavigation } from '../lib/navigation/navigation'
-import { View } from '../lib/navigation/view'
-
-const mockView = (id = 'view', order = 1) => new View({ id, order, name: 'View', icon: '<svg></svg>', getContents: () => Promise.reject(new Error()) })
+import { mockView } from './view.spec'
 
 describe('getNavigation', () => {
 	it('creates a new navigation if needed', () => {
@@ -40,23 +38,31 @@ describe('getNavigation', () => {
 describe('Navigation', () => {
 	it('Can register a view', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		navigation.register(view)
 
 		expect(navigation.views).toEqual([view])
 	})
 
+	it('Throws when trying to register invalid view', async () => {
+		const navigation = new Navigation()
+		expect(() => {
+			// @ts-expect-error mocking to test invalid input
+			navigation.register({ id: 'someid' })
+		}).toThrowError()
+	})
+
 	it('Throws when registering the same view twice', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		navigation.register(view)
 		expect(() => navigation.register(view)).toThrow(/already registered/)
 		expect(navigation.views).toEqual([view])
 	})
 
 	it('Emits update event after registering a view', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		const listener = vi.fn()
 
 		navigation.addEventListener('update', listener)
@@ -68,7 +74,7 @@ describe('Navigation', () => {
 
 	it('Can remove a view', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		navigation.register(view)
 		expect(navigation.views).toEqual([view])
 		navigation.remove(view.id)
@@ -77,7 +83,7 @@ describe('Navigation', () => {
 
 	it('Emits update event after removing a view', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		const listener = vi.fn()
 		navigation.register(view)
 		navigation.addEventListener('update', listener)
@@ -98,7 +104,7 @@ describe('Navigation', () => {
 
 	it('Can set a view as active', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		navigation.register(view)
 
 		expect(navigation.active).toBe(null)
@@ -109,7 +115,7 @@ describe('Navigation', () => {
 
 	it('Emits event when setting a view as active', async () => {
 		const navigation = new Navigation()
-		const view = mockView()
+		const { view } = mockView()
 		navigation.register(view)
 
 		// add listener

__tests__/view.spec.ts

@@ -161,31 +161,7 @@
 
 describe('View creation', () => {
 	test('Create a View', async () => {
-		const folder = new Folder({
-			source: 'https://example.org/dav/files/admin/',
-			root: '/files/admin',
-			owner: 'admin',
-		})
-		const view = new View({
-			id: 'test',
-			name: 'Test',
-			caption: 'Test caption',
-			emptyTitle: 'Test empty title',
-			emptyCaption: 'Test empty caption',
-			getContents: () => Promise.resolve({ folder, contents: [] }),
-			hidden: true,
-			icon: '<svg></svg>',
-			order: 1,
-			params: {},
-			columns: [],
-			emptyView: () => {},
-			parent: 'parent',
-			sticky: false,
-			expanded: false,
-			defaultSortKey: 'key',
-			loadChildViews: async () => {},
-		})
-
+		const { view, folder } = mockView()
 		expect(view.id).toBe('test')
 		expect(view.name).toBe('Test')
 		expect(view.caption).toBe('Test caption')
@@ -205,3 +181,33 @@
 		await expect(view.loadChildViews?.({} as unknown as View)).resolves.toBe(undefined)
 	})
 })
+
+export function mockView() {
+	const folder = new Folder({
+		source: 'https://example.org/dav/files/admin/',
+		root: '/files/admin',
+		owner: 'admin',
+	})
+
+	const view = new View({
+		id: 'test',
+		name: 'Test',
+		caption: 'Test caption',
+		emptyTitle: 'Test empty title',
+		emptyCaption: 'Test empty caption',
+		getContents: () => Promise.resolve({ folder, contents: [] }),
+		hidden: true,
+		icon: '<svg></svg>',
+		order: 1,
+		params: {},
+		columns: [],
+		emptyView: () => {},
+		parent: 'parent',
+		sticky: false,
+		expanded: false,
+		defaultSortKey: 'key',
+		loadChildViews: async () => {},
+	})
+
+	return { folder, view }
+}

lib/fileListHeaders.ts

@@ -3,8 +3,8 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { Folder } from './node/folder.ts'
-import type { View } from './navigation/view.ts'
+import type { IFolder } from './node/folder.ts'
+import type { IView } from './navigation/view.ts'
 
 import logger from './utils/logger.ts'
 
@@ -14,11 +14,11 @@ export interface HeaderData {
 	/** Order */
 	order: number
 	/** Condition wether this header is shown or not */
-	enabled?: (folder: Folder, view: View) => boolean
+	enabled?: (folder: IFolder, view: IView) => boolean
 	/** Executed when file list is initialized */
-	render: (el: HTMLElement, folder: Folder, view: View) => void
+	render: (el: HTMLElement, folder: IFolder, view: IView) => void
 	/** Executed when root folder changed */
-	updated(folder: Folder, view: View)
+	updated(folder: IFolder, view: IView)
 }
 
 export class Header {

lib/navigation/column.ts

@@ -3,23 +3,23 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import type { Node } from '../node/node.ts'
-import type { View } from './view.ts'
+import type { INode } from '../node/node.ts'
+import type { IView } from './view.ts'
 
 interface ColumnData {
 	/** Unique column ID */
 	id: string
 	/** Translated column title */
 	title: string
 	/** The content of the cell. The element will be appended within */
-	render: (node: Node, view: View) => HTMLElement
-	/** Function used to sort Nodes between them */
-	sort?: (nodeA: Node, nodeB: Node) => number
+	render: (node: INode, view: IView) => HTMLElement
+	/** Function used to sort INodes between them */
+	sort?: (nodeA: INode, nodeB: INode) => number
 	/**
 	 * Custom summary of the column to display at the end of the list.
 	 * Will not be displayed if  nothing is provided
 	 */
-	summary?: (node: Node[], view: View) => string
+	summary?: (node: INode[], view: IView) => string
 }
 
 export class Column implements ColumnData {

lib/navigation/navigation.ts

@@ -2,15 +2,18 @@
  * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-import type { View } from './view'
+
+import type { IView } from './view'
+
+import { validateView } from './view'
 import { TypedEventTarget } from 'typescript-event-target'
 import logger from '../utils/logger'
 
 /**
  * The event is emitted when the navigation view was updated.
  * It contains the new active view in the `detail` attribute.
  */
-interface UpdateActiveViewEvent extends CustomEvent<View | null> {
+interface UpdateActiveViewEvent extends CustomEvent<IView | null> {
 	type: 'updateActive'
 }
 
@@ -44,19 +47,22 @@ interface UpdateViewsEvent extends CustomEvent<never> {
  */
 export class Navigation extends TypedEventTarget<{ updateActive: UpdateActiveViewEvent, update: UpdateViewsEvent }> {
 
-	private _views: View[] = []
-	private _currentView: View | null = null
+	private _views: IView[] = []
+	private _currentView: IView | null = null
 
 	/**
 	 * Register a new view on the navigation
 	 * @param view The view to register
-	 * @throws `Error` is thrown if a view with the same id is already registered
+	 * @throws {Error} if a view with the same id is already registered
+	 * @throws {Error} if the registered view is invalid
 	 */
-	register(view: View): void {
+	register(view: IView): void {
 		if (this._views.find(search => search.id === view.id)) {
-			throw new Error(`View id ${view.id} is already registered`)
+			throw new Error(`IView id ${view.id} is already registered`)
 		}
 
+		validateView(view)
+
 		this._views.push(view)
 		this.dispatchTypedEvent('update', new CustomEvent<never>('update') as UpdateViewsEvent)
 	}
@@ -78,23 +84,23 @@ export class Navigation extends TypedEventTarget<{ updateActive: UpdateActiveVie
 	 * @fires UpdateActiveViewEvent
 	 * @param view New active view
 	 */
-	setActive(view: View | null): void {
+	setActive(view: IView | null): void {
 		this._currentView = view
-		const event = new CustomEvent<View | null>('updateActive', { detail: view })
+		const event = new CustomEvent<IView | null>('updateActive', { detail: view })
 		this.dispatchTypedEvent('updateActive', event as UpdateActiveViewEvent)
 	}
 
 	/**
 	 * The currently active files view
 	 */
-	get active(): View | null {
+	get active(): IView | null {
 		return this._currentView
 	}
 
 	/**
 	 * All registered views
 	 */
-	get views(): View[] {
+	get views(): IView[] {
 		return this._views
 	}
 

lib/navigation/view.ts

@@ -14,7 +14,7 @@ export type ContentsWithRoot = {
 	contents: Node[]
 }
 
-interface ViewData {
+export interface IView {
 	/** Unique view ID */
 	id: string
 	/** Translated view name */
@@ -97,12 +97,12 @@ interface ViewData {
 	loadChildViews?: (view: View) => Promise<void>
 }
 
-export class View implements ViewData {
+export class View implements IView {
 
-	private _view: ViewData
+	private _view: IView
 
-	constructor(view: ViewData) {
-		isValidView(view)
+	constructor(view: IView) {
+		validateView(view)
 		this._view = view
 	}
 
@@ -193,14 +193,12 @@ export class View implements ViewData {
 }
 
 /**
- * Typescript cannot validate an interface.
- * Please keep in sync with the View interface requirements.
+ * Validate a view interface to check all required properties are satisfied.
  *
- * @param {ViewData} view the view to check
- * @return {boolean} true if the column is valid
+ * @param view the view to check
  * @throws {Error} if the view is not valid
  */
-const isValidView = function(view: ViewData): boolean {
+export function validateView(view: IView) {
 	if (!view.id || typeof view.id !== 'string') {
 		throw new Error('View id is required and must be a string')
 	}
@@ -261,6 +259,4 @@ const isValidView = function(view: ViewData): boolean {
 	if (view.loadChildViews && typeof view.loadChildViews !== 'function') {
 		throw new Error('View loadChildViews must be a function')
 	}
-
-	return true
 }

lib/types.ts

@@ -3,25 +3,25 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
-import { Folder, Node } from './node/index.ts'
-import { View } from './navigation/index.ts'
+import type { IFolder, INode } from './node/index.ts'
+import type { IView } from './navigation/index.ts'
 
 export type ActionContextSingle = {
-	nodes: [Node],
-	view: View,
-	folder: Folder,
-	contents: Node[],
+	nodes: [INode],
+	view: IView,
+	folder: IFolder,
+	contents: INode[],
 }
 
 export type ActionContext = {
-	nodes: Node[],
-	view: View,
-	folder: Folder,
-	contents: Node[],
+	nodes: INode[],
+	view: IView,
+	folder: IFolder,
+	contents: INode[],
 }
 
 export type ViewActionContext = {
-	view: View,
-	folder: Folder,
-	contents: Node[],
+	view: IView,
+	folder: IFolder,
+	contents: INode[],
 }