Skip to content

Commit

Permalink
feat: add warn/error message infrastructure (#5776)
Browse files Browse the repository at this point in the history
* feat: add warn/error message infrastructure

* Change files

* feat: add sideEffects files and basic debug tests

Co-authored-by: EisenbergEffect <[email protected]>
  • Loading branch information
2 people authored and nicholasrice committed May 9, 2022
1 parent da2af7e commit d57a864
Show file tree
Hide file tree
Showing 13 changed files with 144 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: add warn/error message infrastructure",
"packageName": "@microsoft/fast-element",
"email": "[email protected]",
"dependentChangeType": "patch"
}
8 changes: 7 additions & 1 deletion packages/web-components/fast-element/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,11 @@ export class AttributeDefinition implements Accessor {
setValue(source: HTMLElement, newValue: any): void;
}

// Warning: (ae-forgotten-export) The symbol "reflectMode" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "booleanMode" needs to be exported by the entry point index.d.ts
//
// @public
export type AttributeMode = "reflect" | "boolean" | "fromView";
export type AttributeMode = typeof reflectMode | typeof booleanMode | "fromView";

// @public
export interface Behavior<TSource = any, TParent = any, TGrandparent = any> {
Expand Down Expand Up @@ -340,10 +343,13 @@ export class FASTElementDefinition<TType extends Function = Function> {
//
// @internal
export interface FASTGlobal {
addMessages(messages: Record<number, string>): void;
error(code: number, ...args: any[]): Error;
getById<T>(id: string | number): T | null;
// (undocumented)
getById<T>(id: string | number, initialize: () => T): T;
readonly versions: string[];
warn(code: number, ...args: any[]): void;
}

// @public
Expand Down
5 changes: 4 additions & 1 deletion packages/web-components/fast-element/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"name": "@microsoft/fast-element",
"description": "A library for constructing Web Components",
"sideEffects": false,
"version": "1.10.1",
"author": {
"name": "Microsoft",
Expand All @@ -20,6 +19,10 @@
"types": "dist/fast-element.d.ts",
"type": "module",
"unpkg": "dist/fast-element.min.js",
"sideEffects": [
"./dist/esm/debug.js",
"./dist/esm/polyfills.js"
],
"scripts": {
"clean:dist": "node ../../../build/clean.js dist",
"doc": "api-extractor run --local",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ export interface ValueConverter {
fromView(value: any): any;
}

const booleanMode = "boolean";
const reflectMode = "reflect";

/**
* The mode that specifies the runtime behavior of the attribute.
* @remarks
Expand All @@ -33,7 +36,7 @@ export interface ValueConverter {
* changes in the DOM, but does not reflect property changes back.
* @public
*/
export type AttributeMode = "reflect" | "boolean" | "fromView";
export type AttributeMode = typeof reflectMode | typeof booleanMode | "fromView";

/**
* Metadata used to configure a custom attribute's behavior.
Expand Down Expand Up @@ -149,7 +152,7 @@ export class AttributeDefinition implements Accessor {
Owner: Function,
name: string,
attribute: string = name.toLowerCase(),
mode: AttributeMode = "reflect",
mode: AttributeMode = reflectMode,
converter?: ValueConverter
) {
this.Owner = Owner;
Expand All @@ -161,7 +164,7 @@ export class AttributeDefinition implements Accessor {
this.callbackName = `${name}Changed`;
this.hasCallback = this.callbackName in Owner.prototype;

if (mode === "boolean" && converter === void 0) {
if (mode === booleanMode && converter === void 0) {
this.converter = booleanConverter;
}
}
Expand Down Expand Up @@ -226,15 +229,15 @@ export class AttributeDefinition implements Accessor {
const latestValue = element[this.fieldName];

switch (mode) {
case "reflect":
case reflectMode:
const converter = this.converter;
DOM.setAttribute(
element,
this.attribute,
converter !== void 0 ? converter.toView(latestValue) : latestValue
);
break;
case "boolean":
case booleanMode:
DOM.setBooleanAttribute(element, this.attribute, latestValue);
break;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { Mutable, StyleTarget } from "../interfaces.js";
import { Message, Mutable, StyleTarget } from "../interfaces.js";
import type { Behavior } from "../observation/behavior.js";
import { PropertyChangeNotifier } from "../observation/notifier.js";
import { defaultExecutionContext, Observable } from "../observation/observable.js";
import { FAST } from "../platform.js";
import type { ElementStyles } from "../styles/element-styles.js";
import type { ElementViewTemplate } from "../templating/template.js";
import type { ElementView } from "../templating/view.js";
Expand All @@ -18,6 +19,8 @@ function getShadowRoot(element: HTMLElement): ShadowRoot | null {
return element.shadowRoot ?? shadowRoots.get(element) ?? null;
}

const isConnectedPropertyName = "isConnected";

/**
* Controls the lifecycle and rendering of a `FASTElement`.
* @public
Expand Down Expand Up @@ -64,13 +67,13 @@ export class Controller extends PropertyChangeNotifier {
* connected to the document.
*/
public get isConnected(): boolean {
Observable.track(this, "isConnected");
Observable.track(this, isConnectedPropertyName);
return this._isConnected;
}

private setIsConnected(value: boolean): void {
this._isConnected = value;
Observable.notify(this, "isConnected");
Observable.notify(this, isConnectedPropertyName);
}

/**
Expand Down Expand Up @@ -474,7 +477,7 @@ export class Controller extends PropertyChangeNotifier {
const definition = FASTElementDefinition.forType(element.constructor);

if (definition === void 0) {
throw new Error("Missing FASTElement definition.");
throw FAST.error(Message.missingElementDefinition);
}

return ((element as any).$fastController = new Controller(element, definition));
Expand Down
20 changes: 20 additions & 0 deletions packages/web-components/fast-element/src/debug.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { expect } from "chai";
import "./debug.js";
import { FASTGlobal, Message } from "./interfaces";

declare const FAST: FASTGlobal;

describe("The debug module", () => {
context("when sending errors", () => {
it("expect known error message from known error code", () => {
const error = FAST.error(Message.bindingInnerHTMLRequiresTrustedTypes);
expect(error.message.length).greaterThan(0);
expect(error.message).not.equal("Unknown Error");
});

it("expect unknown error message from unknown error code", () => {
const error = FAST.error(10);
expect(error.message).equal("Unknown Error");
});
});
});
31 changes: 31 additions & 0 deletions packages/web-components/fast-element/src/debug.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { FASTGlobal } from "./interfaces.js";

if (globalThis.FAST === void 0) {
Reflect.defineProperty(globalThis, "FAST", {
value: Object.create(null),
configurable: false,
enumerable: false,
writable: false,
});
}

const FAST: FASTGlobal = globalThis.FAST;

const debugMessages = {
[1101 /* needsArrayObservation */]: "Must call enableArrayObservation before observing arrays.",
[1201 /* onlySetHTMLPolicyOnce */]: "The HTML policy can only be set once.",
[1202 /* bindingInnerHTMLRequiresTrustedTypes */]: "To bind innerHTML, you must use a TrustedTypesPolicy.",
[1401 /* missingElementDefinition */]: "Missing FASTElement definition.",
};

Object.assign(FAST, {
addMessages(messages: Record<number, string>) {
Object.assign(debugMessages, messages);
},
warn(code: number, ...args: any[]) {
console.warn(debugMessages[code] ?? "Unknown Warning");
},
error(code: number, ...args: any[]) {
return new Error(debugMessages[code] ?? "Unknown Error");
},
});
37 changes: 37 additions & 0 deletions packages/web-components/fast-element/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,26 @@ export interface FASTGlobal {
*/
getById<T>(id: string | number): T | null;
getById<T>(id: string | number, initialize: () => T): T;

/**
* Sends a warning to the developer.
* @param code - The warning code to send.
* @param args - Args relevant for the warning.
*/
warn(code: number, ...args: any[]): void;

/**
* Creates an error.
* @param code - The error code to send.
* @param args - Args relevant for the error.
*/
error(code: number, ...args: any[]): Error;

/**
* Adds debug messages for errors and warnings.
* @param messages - The message dictionary to add.
*/
addMessages(messages: Record<number, string>): void;
}

/**
Expand All @@ -75,6 +95,7 @@ export const enum KernelServiceId {
contextEvent = 3,
elementRegistry = 4,
styleSheetStrategy = 5,
developerChannel = 6,
}

/**
Expand Down Expand Up @@ -125,6 +146,22 @@ export interface StyleStrategy {
removeStylesFrom(target: StyleTarget): void;
}

/**
* Warning and error messages.
* @internal
*/
export const enum Message {
// 1000 - 1100 Kernel
// 1101 - 1200 Observation
needsArrayObservation = 1101,
// 1201 - 1300 Templating
onlySetHTMLPolicyOnce = 1201,
bindingInnerHTMLRequiresTrustedTypes = 1202,
// 1301 - 1400 Styles
// 1401 - 1500 Components
missingElementDefinition = 1401,
}

/**
* @internal
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DOM } from "../dom.js";
import { isFunction, isString, KernelServiceId } from "../interfaces.js";
import { isFunction, isString, KernelServiceId, Message } from "../interfaces.js";
import { FAST } from "../platform.js";
import { PropertyChangeNotifier, SubscriberSet } from "./notifier.js";
import type { Notifier, Subscriber } from "./notifier.js";
Expand Down Expand Up @@ -97,7 +97,7 @@ export const Observable = FAST.getById(KernelServiceId.observable, () => {
const accessorLookup = new WeakMap<any, Accessor[]>();
let watcher: BindingObserverImplementation | undefined = void 0;
let createArrayObserver = (array: any[]): Notifier => {
throw new Error("Must call enableArrayObservation before observing arrays.");
throw FAST.error(Message.needsArrayObservation);
};

function getNotifier(source: any): Notifier {
Expand Down
12 changes: 11 additions & 1 deletion packages/web-components/fast-element/src/platform.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { FASTGlobal } from "./interfaces.js";

// ensure FAST global - duplicated in polyfills.ts
// ensure FAST global - duplicated in polyfills.ts and debug.ts
const propConfig = {
configurable: false,
enumerable: false,
Expand Down Expand Up @@ -37,6 +37,16 @@ if (FAST.getById === void 0) {
});
}

if (FAST.error === void 0) {
Object.assign(FAST, {
warn() {},
error(code: number) {
return new Error(`Code ${code}`);
},
addMessages() {},
});
}

/**
* A readonly, empty array.
* @remarks
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { DOM } from "../dom.js";
import { isString, Mutable } from "../interfaces.js";
import { isString, Message, Mutable } from "../interfaces.js";
import {
Binding,
BindingObserver,
ExecutionContext,
Observable,
} from "../observation/observable.js";
import { FAST } from "../platform.js";
import {
Aspect,
AspectedHTMLDirective,
Expand Down Expand Up @@ -502,7 +503,7 @@ const createInnerHTMLBinding = globalThis.TrustedHTML
return value;
}

throw new Error("To bind innerHTML, you must use a TrustedTypesPolicy.");
throw FAST.error(Message.bindingInnerHTMLRequiresTrustedTypes);
}
: (binding: Binding) => binding;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { isString, TrustedTypesPolicy } from "../interfaces.js";
import { isString, Message, TrustedTypesPolicy } from "../interfaces.js";
import type { ExecutionContext } from "../observation/observable.js";
import { FAST } from "../platform.js";
import { Parser } from "./markup.js";
import { bind, oneTime } from "./binding.js";
import type {
Expand Down Expand Up @@ -286,7 +287,7 @@ export const Compiler = {
*/
setHTMLPolicy(policy: TrustedTypesPolicy) {
if (htmlPolicy !== fastHTMLPolicy) {
throw new Error("The HTML policy can only be set once.");
throw FAST.error(Message.onlySetHTMLPolicyOnce);
}

htmlPolicy = policy;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { isString } from "../interfaces.js";
import { NodeBehaviorOptions, NodeObservationDirective } from "./node-observation.js";
import type { CaptureType } from "./template.js";

const slotEvent = "slotchange";

/**
* The options used to configure slotted node observation.
* @public
Expand All @@ -20,15 +22,15 @@ export class SlottedDirective extends NodeObservationDirective<SlottedDirectiveO
* @param target - The target to observe.
*/
observe(target: EventSource): void {
target.addEventListener("slotchange", this);
target.addEventListener(slotEvent, this);
}

/**
* Disconnects observation of the nodes.
* @param target - The target to unobserve.
*/
disconnect(target: EventSource): void {
target.removeEventListener("slotchange", this);
target.removeEventListener(slotEvent, this);
}

/**
Expand Down

0 comments on commit d57a864

Please sign in to comment.