Skip to content

Commit

Permalink
fix: refine re-evaluations error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
weareoutman committed Aug 6, 2020
1 parent d33738c commit 11fd9f1
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 35 deletions.
28 changes: 27 additions & 1 deletion src/hook/dehydrate.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dehydrate } from "./dehydrate";
import { dehydrate, restoreDehydrated } from "./dehydrate";
import { PROP_DEHYDRATED } from "../shared/constants";

describe("dehydrate", () => {
Expand Down Expand Up @@ -207,3 +207,29 @@ describe("dehydrate", () => {
}
);
});

describe("restoreDehydrated", () => {
it("should restore dehydrated CustomEvent", () => {
const dehydratedCustomEvent = dehydrate(
new CustomEvent("something.happen", {
detail: {
quality: "good",
},
}),
[]
);
const restoredCustomEvent = restoreDehydrated(dehydratedCustomEvent);
expect(restoredCustomEvent instanceof CustomEvent).toBe(true);
expect(restoredCustomEvent.type).toBe("something.happen");
expect(restoredCustomEvent.detail).toEqual({
quality: "good",
});
});

it("should restore dehydrated Event", () => {
const dehydratedCustomEvent = dehydrate(new Event("something.happen"), []);
const restoredCustomEvent = restoreDehydrated(dehydratedCustomEvent);
expect(restoredCustomEvent instanceof Event).toBe(true);
expect(restoredCustomEvent.type).toBe("something.happen");
});
});
26 changes: 23 additions & 3 deletions src/hook/dehydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ import { PROP_DEHYDRATED } from "../shared/constants";
* @param repo repository to collect circular references.
* @returns original value if it's serializable, or a dehydrated wrapper if not.
*/
export function dehydrate(value: any, repo: any[], memo = new WeakMap()): any {
if (memo.has(value)) {
const processed = memo.get(value);
export function dehydrate(
value: unknown,
repo: any[],
memo = new WeakMap()
): any {
if (memo.has(value as any)) {
const processed = memo.get(value as any);
let repoIndex = repo.indexOf(processed);
if (repoIndex < 0) {
repoIndex = repo.length;
Expand Down Expand Up @@ -95,6 +99,22 @@ export function dehydrate(value: any, repo: any[], memo = new WeakMap()): any {
return value;
}

// This API is exposed to Brick Next itself, keep compatible.
export function restoreDehydrated(value: unknown): any {
const dehydrated: Dehydrated = (value as any)?.[PROP_DEHYDRATED];
if (dehydrated && dehydrated.type === "object" && dehydrated.children) {
if (dehydrated.constructorName === "CustomEvent") {
return new CustomEvent(dehydrated.children.type, {
detail: dehydrated.children.detail,
});
}
if (dehydrated.constructorName === "Event") {
return new Event(dehydrated.children.type);
}
}
return value;
}

function wrapDehydrated(data: Dehydrated): DehydratedWrapper {
return {
[PROP_DEHYDRATED]: data,
Expand Down
1 change: 1 addition & 0 deletions src/hook/emit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { dehydrate } from "./dehydrate";
import { EmitData } from "../shared/interfaces";
import { MESSAGE_SOURCE_HOOK } from "../shared/constants";

// This API is exposed to Brick Next itself, keep compatible.
export function emit(data: EmitData): void {
try {
const repo: any[] = [];
Expand Down
5 changes: 4 additions & 1 deletion src/hook/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getBricks, getBrickByUid, getBrickInfo } from "./traverse";
import { inspectElement, dismissInspections } from "./inspector";
import { emit } from "./emit";
import { overrideProps } from "./overrideProps";
import { restoreDehydrated } from "./dehydrate";

function injectHook(): void {
if (Object.prototype.hasOwnProperty.call(window, HOOK_NAME)) {
Expand All @@ -15,7 +16,6 @@ function injectHook(): void {
getBrickInfo,
inspectBrick: (uid: number) => inspectElement(getBrickByUid(uid)),
dismissInspections,
emit,
overrideProps: (uid: number, propertyName: string, value: any) =>
overrideProps(getBrickByUid(uid), propertyName, value),
supports: (...features: string[]) =>
Expand All @@ -24,6 +24,9 @@ function injectHook(): void {
(window as any).BRICK_NEXT_FEATURES.includes(item)
)
: false,
// Methods below are exposed to Brick Next itself, keep compatible.
emit,
restoreDehydrated,
};

Object.defineProperty(hook, "pageHasBricks", {
Expand Down
17 changes: 14 additions & 3 deletions src/panel/components/EvaluationsPanel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const savePreserveLogs = jest.fn();
(useEvaluationsContext as jest.Mock).mockReturnValue({
evaluations: [
{
id: 1,
detail: {
raw: "<% EVENT.detail %>",
result: "good",
Expand All @@ -23,7 +24,6 @@ const savePreserveLogs = jest.fn();
},
},
},
id: 0,
},
{
detail: {
Expand All @@ -36,6 +36,14 @@ const savePreserveLogs = jest.fn();
},
},
},
{
id: 2,
detail: {
raw: "<% DATA.quality %>",
context: {},
},
error: "DATA is undefined",
},
],
setEvaluations,
savePreserveLogs,
Expand All @@ -48,7 +56,10 @@ describe("EvaluationsPanel", () => {

it("should work", () => {
const wrapper = shallow(<EvaluationsPanel />);
expect(wrapper.find("tbody").find("tr").length).toBe(2);
expect(wrapper.find("tbody").find("tr").length).toBe(3);
expect(wrapper.find("tbody").find("tr").at(2).find("td").at(1).text()).toBe(
"Error: DATA is undefined"
);
});

it("should toggle string-wrap", () => {
Expand Down Expand Up @@ -111,7 +122,7 @@ describe("EvaluationsPanel", () => {
detail: "good",
},
},
id: 0,
id: 1,
raw: "<% DATA.name %>",
type: "devtools-evaluation-edit",
},
Expand Down
8 changes: 7 additions & 1 deletion src/panel/components/EvaluationsPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,13 @@ export function EvaluationsPanel(): React.ReactElement {
/>
</td>
<td>
<PropItem propValue={item.detail?.result} standalone />
{item.error ? (
<code className="bp3-code error-message">
Error: {item.error}
</code>
) : (
<PropItem propValue={item.detail?.result} standalone />
)}
</td>
<td>
<PropList list={item.detail?.context || {}} />
Expand Down
25 changes: 10 additions & 15 deletions src/panel/components/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,12 @@ export function Layout(): React.ReactElement {
((data = event.data.payload), data?.type === "re-evaluation")
) {
const value = hydrate(data.payload, data.repo);
const { id, raw, result } = value;
setEvaluations((prev) => {
const selected = prev.find((item) => item.id === id);
selected &&
Object.assign(selected.detail, {
raw,
result: result.error ? `ERROR: ${result.error}` : result.data,
});

const selected = prev.find((item) => item.id === value.id);
if (selected) {
selected.detail = value.detail;
selected.error = value.error;
}
return [...prev];
});
}
Expand Down Expand Up @@ -88,14 +85,12 @@ export function Layout(): React.ReactElement {
((data = event.data.payload), data?.type === "re-transformation")
) {
const value = hydrate(data.payload, data.repo);
const { id, transform, result } = value;
setTransformations((prev) => {
const selected = prev.find((item) => item.id === id);
selected &&
Object.assign(selected.detail, {
transform,
result: result.error ? `ERROR: ${result.error}` : result.data,
});
const selected = prev.find((item) => item.id === value.id);
if (selected) {
selected.detail = value.detail;
selected.error = value.error;
}
return [...prev];
});
}
Expand Down
18 changes: 16 additions & 2 deletions src/panel/components/TransformationsPanel.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const savePreserveLogs = jest.fn();
(useTransformationsContext as jest.Mock).mockReturnValue({
transformations: [
{
id: 1,
detail: {
transform: "quality",
result: {
Expand All @@ -22,7 +23,17 @@ const savePreserveLogs = jest.fn();
mapArray: undefined,
},
},
id: 1,
},
{
id: 2,
detail: {
transform: {
quality: "<% DATA() %>",
},
data: "good",
options: {},
},
error: "DATA is not a function",
},
],
setTransformations,
Expand All @@ -36,7 +47,10 @@ describe("TransformationsPanel", () => {

it("should work", () => {
const wrapper = shallow(<TransformationsPanel />);
expect(wrapper.find("tbody").find("tr").length).toBe(1);
expect(wrapper.find("tbody").find("tr").length).toBe(2);
expect(wrapper.find("tbody").find("tr").at(1).find("td").at(1).text()).toBe(
"Error: DATA is not a function"
);
});

it("should toggle string-wrap", () => {
Expand Down
8 changes: 7 additions & 1 deletion src/panel/components/TransformationsPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ export function TransformationsPanel(): React.ReactElement {
/>
</td>
<td>
<PropItem propValue={item.detail?.result} standalone />
{item.error ? (
<code className="bp3-code error-message">
Error: {item.error}
</code>
) : (
<PropItem propValue={item.detail?.result} standalone />
)}
</td>
<td>
<PropItem propValue={item.detail?.data} standalone />
Expand Down
4 changes: 2 additions & 2 deletions src/panel/libs/hydrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ function noop(): void {
* @returns re-hydrated data
*/
export function hydrate(
value: any,
value: unknown,
repo: any[],
memo = new Map<number, any>(),
ref?: number
): any {
const dehydrated: Dehydrated = value?.[PROP_DEHYDRATED];
const dehydrated: Dehydrated = (value as any)?.[PROP_DEHYDRATED];
if (dehydrated) {
switch (dehydrated.type) {
case "ref":
Expand Down
8 changes: 4 additions & 4 deletions src/panel/libs/utils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { PROP_DEHYDRATED } from "../../shared/constants";
import { DehydratedWrapper } from "../../shared/interfaces";

export function isDehydrated(value: any): value is DehydratedWrapper {
return !!value?.[PROP_DEHYDRATED];
export function isDehydrated(value: unknown): value is DehydratedWrapper {
return !!(value as any)?.[PROP_DEHYDRATED];
}

export function isObject(value: any): value is Record<string, any> {
return typeof value === "object" && value;
export function isObject(value: unknown): value is Record<string, any> {
return typeof value === "object" && !!value;
}
5 changes: 5 additions & 0 deletions src/panel/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,8 @@ body {
.table-view td > .prop-list {
padding-left: 0;
}

.bp3-code.error-message {
color: #f5498b;
font-size: inherit;
}
6 changes: 4 additions & 2 deletions src/shared/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@ export interface EvaluationDetail {
}

export interface Evaluation {
id: number;
detail: EvaluationDetail;
id?: number;
error?: string;
}

export interface Transformation {
id: number;
detail: TransformationDetail;
id?: number;
error?: string;
}

export interface TransformationDetail {
Expand Down

0 comments on commit 11fd9f1

Please sign in to comment.