Skip to content

Commit

Permalink
fix(Application): Fix "notifyErrorAndThrow" not working with a short-…
Browse files Browse the repository at this point in the history
…hand parameter
  • Loading branch information
a11delavar committed Jan 19, 2024
1 parent 030e264 commit 749f893
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 32 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions packages/Application/Dialog/DialogComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ export enum DialogConfirmationStrategy { Dialog, Tab, Window }

export type PopupConfirmationStrategy = Exclude<DialogConfirmationStrategy, DialogConfirmationStrategy.Dialog>

const dialogElementConstructorSymbol = Symbol('DialogComponent.DialogElementConstructor')

export abstract class DialogComponent<T extends DialogParameters = void, TResult = void> extends Component {
static readonly connectingHooks = new HookSet<DialogComponent<any, any>>()

private static readonly dialogElementConstructorSymbol = Symbol('DialogComponent.DialogElementConstructor')

static dialogElement() {
return (constructor: Constructor<Dialog>) => {
(constructor as any)[DialogComponent.dialogElementConstructorSymbol] = true
(constructor as any)[dialogElementConstructorSymbol] = true
}
}

Expand All @@ -30,7 +30,7 @@ export abstract class DialogComponent<T extends DialogParameters = void, TResult
return Promise.resolve(Application.topLayer)
}

@querySymbolizedElement(DialogComponent.dialogElementConstructorSymbol) readonly dialogElement!: Dialog & HTMLElement
@querySymbolizedElement(dialogElementConstructorSymbol) readonly dialogElement!: Dialog & HTMLElement

get primaryActionElement() { return this.dialogElement.primaryActionElement }

Expand Down
97 changes: 97 additions & 0 deletions packages/Application/Notification/NotificationComponent.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { ComponentTestFixture } from '@a11d/lit-testing'
import { Notification, NotificationComponent, NotificationType } from './NotificationComponent.js'
import { component } from '@a11d/lit'

describe('NotificationComponent', () => {
@NotificationComponent.defaultComponent()
@component('test-notification')
class TestNotificationComponent extends NotificationComponent {
override notification!: Notification

override show() {
return Promise.resolve()
}
}

const fixture = new ComponentTestFixture(() => new TestNotificationComponent())
fixture

const all = [
{ method: 'notifyInfo', type: NotificationType.Info },
{ method: 'notifySuccess', type: NotificationType.Success },
{ method: 'notifyWarning', type: NotificationType.Warning },
{ method: 'notifyError', type: NotificationType.Error },
] as const

const action = { title: 'Test', handleClick: () => { } }

for (const { method, type } of all) {

it(`should proxy "${method}" with shortened parameters to "notify" with correct parameters`, async () => {
spyOn(TestNotificationComponent, 'notify').and.callThrough()

await TestNotificationComponent[method]('Test', action)

expect(TestNotificationComponent.notify).toHaveBeenCalledWith({
type,
message: 'Test',
actions: [action],
})
})

it(`should proxy "${method}" with full parameters to "notify" with correct parameters`, async () => {
spyOn(TestNotificationComponent, 'notify').and.callThrough()

await TestNotificationComponent[method]({
message: 'Test',
actions: [action],
})

expect(TestNotificationComponent.notify).toHaveBeenCalledWith({
type,
message: 'Test',
actions: [action],
})
})
}

describe('notifyAndThrowError', () => {
it('should throw and proxy on shorthand parameters with error', () => {
spyOn(TestNotificationComponent, 'notify').and.callThrough()
const error = new Error('Test')

expect(() => TestNotificationComponent.notifyAndThrowError(error, action)).toThrow(error)
expect(TestNotificationComponent.notify).toHaveBeenCalledWith({
type: NotificationType.Error,
message: 'Test',
actions: [action],
})
})

it('should throw and proxy on shorthand parameters with error-message', () => {
spyOn(TestNotificationComponent, 'notify').and.callThrough()
const errorMessage = 'Test'

expect(() => TestNotificationComponent.notifyAndThrowError(errorMessage, action)).toThrow(new Error(errorMessage))
expect(TestNotificationComponent.notify).toHaveBeenCalledWith({
type: NotificationType.Error,
message: errorMessage,
actions: [action],
})
})

it('should throw and proxy on shorthand parameters with notification parameters', () => {
spyOn(TestNotificationComponent, 'notify').and.callThrough()

expect(() => TestNotificationComponent.notifyAndThrowError({
message: 'Test',
actions: [action],
})).toThrow(new Error('Test'))
expect(TestNotificationComponent.notify).toHaveBeenCalledWith({
type: NotificationType.Error,
message: 'Test',
actions: [action],
})
})
})
})
46 changes: 27 additions & 19 deletions packages/Application/Notification/NotificationComponent.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { LitElement } from "@a11d/lit"
import { Application } from "../Application.js"
import { NonInertableComponent } from "@a11d/non-inertable-component"
import { LitElement } from '@a11d/lit'
import { Application } from '../Application.js'
import { NonInertableComponent } from '@a11d/non-inertable-component'

export enum NotificationType {
Info = 'info',
Expand All @@ -27,8 +27,9 @@ type NonTypedNotificationParameters =
| [message: string, ...actions: Array<NotificationAction>]

type NonTypedNotificationWithErrorParameters =
| [notification: Partial<NonTypedNotification>]
| [message?: string, ...actions: Array<NotificationAction>]
| [notification: NonTypedNotification]
| [errorMessage: string, ...actions: Array<NotificationAction>]
| [error: Error, ...actions: Array<NotificationAction>]

function normalizeNonTypedNotificationParameters(...parameters: NonTypedNotificationParameters) {
return typeof parameters[0] !== 'string' ? parameters[0] : {
Expand All @@ -48,28 +49,35 @@ export abstract class NotificationComponent extends NonInertableComponent {
}
}

static notifyInfo(...notification: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Info, ...normalizeNonTypedNotificationParameters(...notification) })
static notifyInfo(...parameters: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Info, ...normalizeNonTypedNotificationParameters(...parameters) })
}

static notifySuccess(...notification: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Success, ...normalizeNonTypedNotificationParameters(...notification) })
static notifySuccess(...parameters: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Success, ...normalizeNonTypedNotificationParameters(...parameters) })
}

static notifyWarning(...notification: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Warning, ...normalizeNonTypedNotificationParameters(...notification) })
static notifyWarning(...parameters: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Warning, ...normalizeNonTypedNotificationParameters(...parameters) })
}

static notifyError(...notification: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Error, ...normalizeNonTypedNotificationParameters(...notification) })
static notifyError(...parameters: NonTypedNotificationParameters) {
return this.notify({ type: NotificationType.Error, ...normalizeNonTypedNotificationParameters(...parameters) })
}

static notifyAndThrowError<TError extends Error>(error: TError, ...notification: NonTypedNotificationWithErrorParameters) {
const normalizedNotification = notification.length === 0
? {} as NonTypedNotification
: normalizeNonTypedNotificationParameters(...notification as NonTypedNotificationParameters)
normalizedNotification.message ??= error.message
this.notifyError(normalizedNotification)
static notifyAndThrowError(...parameters: NonTypedNotificationWithErrorParameters) {
let error: Error
if (parameters[0] instanceof Error) {
error = parameters[0]
this.notifyError({ message: error.message, actions: parameters.slice(1) as Array<NotificationAction> })
} else if (typeof parameters[0] === 'string') {
error = new Error(parameters[0])
this.notifyError({ message: error.message, actions: parameters.slice(1) as Array<NotificationAction> })
} else {
const notification = parameters[0]
error = new Error(notification.message)
this.notifyError(notification)
}
throw error
}

Expand Down
8 changes: 4 additions & 4 deletions packages/Application/Page/PageComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ export type PageParameters = void | Record<string, string | number | undefined>

export enum PageNavigationStrategy { Page, Tab, Window }

const pageElementConstructorSymbol = Symbol('PageComponent.PageElementConstructor')

export abstract class PageComponent<T extends PageParameters = void> extends Component {
static readonly connectingHooks = new HookSet<PageComponent<any>>()

private static readonly pageElementConstructorSymbol = Symbol('PageComponent.PageElementConstructor')

static defaultPageElementTag = literal`lit-page`

static pageElement() {
return (constructor: Constructor<Page>) => {
(constructor as any)[PageComponent.pageElementConstructorSymbol] = true
(constructor as any)[pageElementConstructorSymbol] = true
}
}

Expand All @@ -32,7 +32,7 @@ export abstract class PageComponent<T extends PageParameters = void> extends Com
}
}

@querySymbolizedElement(PageComponent.pageElementConstructorSymbol) readonly pageElement!: Page & HTMLElement
@querySymbolizedElement(pageElementConstructorSymbol) readonly pageElement!: Page & HTMLElement

constructor(readonly parameters: T) {
super()
Expand Down
5 changes: 3 additions & 2 deletions packages/Application/PwaHelper/ManifestProvider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Application, type Manifest } from '../index.js'
import { Application } from '../Application.js'
import type { Manifest } from './Manifest.js'

Application.connectingHooks.add(async () => {
Application?.connectingHooks.add(async () => {
// @ts-expect-error - This is declared as a constant property
globalThis.manifest = undefined
const manifestLink = globalThis.document.head.querySelector<HTMLLinkElement>('link[rel=manifest]')
Expand Down
2 changes: 1 addition & 1 deletion packages/Application/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@a11d/lit-application",
"version": "0.1.9",
"version": "0.1.10",
"description": "Lit-based application shell.",
"repository": {
"type": "git",
Expand Down
3 changes: 2 additions & 1 deletion scripts/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ globalThis.environment = 'test'
${getTestFiles('./packages').map(file => {
const relativePath = Path.relative('./dist', file)
return `import '${relativePath.replace(/\\/g, '/').replace(/\.ts$/, '')}'`
}).join('\n')}`
}).join('\n')}
`
FileSystem.writeFileSync('./dist/test.ts', testIndexFileContent)

await esbuild.build({
Expand Down

0 comments on commit 749f893

Please sign in to comment.