From 6ca7c550c6c39fdd422ddb58559d12bd94e19a28 Mon Sep 17 00:00:00 2001 From: MayaGillilan <101652314+MayaGillilan@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:27:22 +0200 Subject: [PATCH 1/3] feat: improve error messaging --- .../src/__tests__/index.spec.ts | 2 +- .../src/__tests__/init.spec.ts | 2 +- .../src/__tests__/react.spec.tsx | 2 +- packages/live-preview-sdk/src/index.ts | 4 ++-- .../__tests__/getAllTaggedElements.bench.ts | 3 +-- .../__tests__/getAllTaggedElements.test.ts | 10 ++++++--- .../src/inspectorMode/utils.ts | 22 +++++++++++++++---- packages/live-preview-sdk/src/liveUpdates.ts | 11 ++++++++++ packages/live-preview-sdk/src/react.tsx | 2 +- 9 files changed, 43 insertions(+), 15 deletions(-) diff --git a/packages/live-preview-sdk/src/__tests__/index.spec.ts b/packages/live-preview-sdk/src/__tests__/index.spec.ts index 50356127..219196de 100644 --- a/packages/live-preview-sdk/src/__tests__/index.spec.ts +++ b/packages/live-preview-sdk/src/__tests__/index.spec.ts @@ -57,7 +57,7 @@ describe('ContentfulLivePreview', () => { describe('init', () => { describe('should bind the message listeners', () => { - it('provide the data to InspectorMode and LiveUpdates', () => { + it.only('provide the data to InspectorMode and LiveUpdates', () => { const data = { source: LIVE_PREVIEW_EDITOR_SOURCE, value: 'any' }; window.dispatchEvent(new MessageEvent('message', { data })); diff --git a/packages/live-preview-sdk/src/__tests__/init.spec.ts b/packages/live-preview-sdk/src/__tests__/init.spec.ts index 4febe67f..5f56781b 100644 --- a/packages/live-preview-sdk/src/__tests__/init.spec.ts +++ b/packages/live-preview-sdk/src/__tests__/init.spec.ts @@ -69,7 +69,7 @@ describe('init', () => { it('warns about invalid init call (missing locale)', () => { expect(ContentfulLivePreview.init).toThrow( - "Init function have to be called with a locale configuration (for example: `ContentfulLivePreview.init({ locale: 'en-US'})`)", + "Init function has to be called with a locale configuration (for example: `ContentfulLivePreview.init({ locale: 'en-US'})`)", ); }); diff --git a/packages/live-preview-sdk/src/__tests__/react.spec.tsx b/packages/live-preview-sdk/src/__tests__/react.spec.tsx index bc5130d6..56ac5633 100644 --- a/packages/live-preview-sdk/src/__tests__/react.spec.tsx +++ b/packages/live-preview-sdk/src/__tests__/react.spec.tsx @@ -24,7 +24,7 @@ describe('ContentfulLivePreviewProvider', () => { // @ts-expect-error -- case locale not provided (e.g. JavaScript usage) () => render(Hello World), ).toThrowError( - 'ContentfulLivePreviewProvider have to be called with a locale property (for example: `{children}`', + 'ContentfulLivePreviewProvider has to be called with a locale property (for example: `{children}`', ); spy.mockRestore(); diff --git a/packages/live-preview-sdk/src/index.ts b/packages/live-preview-sdk/src/index.ts index 9f91a141..00799b90 100644 --- a/packages/live-preview-sdk/src/index.ts +++ b/packages/live-preview-sdk/src/index.ts @@ -92,7 +92,7 @@ export class ContentfulLivePreview { static init(config: ContentfulLivePreviewInitConfig): Promise | undefined { if (typeof config !== 'object' || !config?.locale) { throw new Error( - "Init function have to be called with a locale configuration (for example: `ContentfulLivePreview.init({ locale: 'en-US'})`)", + "Init function has to be called with a locale configuration (for example: `ContentfulLivePreview.init({ locale: 'en-US'})`)", ); } @@ -374,7 +374,7 @@ export class ContentfulLivePreview { return; } - debug.error('Please provide field id and entry id to openEntryInEditor.'); + debug.error('Please provide field id and entry/asset id to openEntryInEditor.'); } /** diff --git a/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.bench.ts b/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.bench.ts index b7862de8..fb60bf6f 100644 --- a/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.bench.ts +++ b/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.bench.ts @@ -91,8 +91,7 @@ describe('getAllTaggedElements', () => { const elements = getAllTaggedElements({ root: dom, - ignoreManual: true, - options: { locale: 'en-US' }, + options: { locale: 'en-US', ignoreManuallyTaggedElements: true }, }); expect(elements).toHaveLength(1 + 10 + 30 + 3); diff --git a/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.test.ts b/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.test.ts index 1e4012e5..38ec48bd 100644 --- a/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.test.ts +++ b/packages/live-preview-sdk/src/inspectorMode/__tests__/getAllTaggedElements.test.ts @@ -1,6 +1,7 @@ import { combine } from '@contentful/content-source-maps'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; +import { setDebugMode } from '../../helpers/debug.js'; import { InspectorModeDataAttributes, InspectorModeEntryAttributes } from '../types.js'; import { getAllTaggedElements } from '../utils.js'; import { createSourceMapFixture } from './fixtures/contentSourceMap.js'; @@ -174,11 +175,15 @@ describe('getAllTaggedElements', () => { `${combine('Test', createSourceMapFixture('ignore_origin', { origin: 'example.com' }))}`, ); + setDebugMode(true); + vi.spyOn(console, 'warn'); + const { taggedElements: elements } = getAllTaggedElements({ root: dom, options: { locale: 'en-US' }, }); expect(elements).toEqual([]); + expect(console.warn).toHaveBeenCalled(); }); it('should recognize auto-tagged elements', () => { @@ -392,8 +397,7 @@ describe('getAllTaggedElements', () => { manuallyTaggedCount, } = getAllTaggedElements({ root: dom, - ignoreManual: true, - options: { locale: 'en-US' }, + options: { locale: 'en-US', ignoreManuallyTaggedElements: true }, }); expect(elements.length).toEqual(1); diff --git a/packages/live-preview-sdk/src/inspectorMode/utils.ts b/packages/live-preview-sdk/src/inspectorMode/utils.ts index 45adf37d..cc448dd1 100644 --- a/packages/live-preview-sdk/src/inspectorMode/utils.ts +++ b/packages/live-preview-sdk/src/inspectorMode/utils.ts @@ -67,6 +67,16 @@ export function getManualInspectorModeAttributes( manuallyTagged: true, }; + if (!sharedProps.fieldId) { + debug.warn('Tagged element is missing field ID attribute and cannot be tagged', { + id: + element.getAttribute(InspectorModeDataAttributes.ENTRY_ID) ?? + element.getAttribute(InspectorModeDataAttributes.ASSET_ID), + sharedProps, + }); + return null; + } + const entryId = element.getAttribute(InspectorModeDataAttributes.ENTRY_ID); if (entryId) { return { ...sharedProps, entryId }; @@ -148,18 +158,16 @@ function hasTaggedParent(node: HTMLElement, taggedElements: TaggedElement[]): bo export function getAllTaggedElements({ root = window.document, options, - ignoreManual, }: { root?: any; options: Omit; - ignoreManual?: boolean; }): { taggedElements: TaggedElement[]; manuallyTaggedCount: number; automaticallyTaggedCount: number; autoTaggedElements: AutoTaggedElement[]; } { - const alreadyTagged = ignoreManual + const alreadyTagged = options.ignoreManuallyTaggedElements ? [] : root.querySelectorAll( `[${InspectorModeDataAttributes.ASSET_ID}][${InspectorModeDataAttributes.FIELD_ID}], [${InspectorModeDataAttributes.ENTRY_ID}][${InspectorModeDataAttributes.FIELD_ID}]`, @@ -180,7 +188,13 @@ export function getAllTaggedElements({ for (const { node, text } of stegaNodes) { const sourceMap = decode(text); if (!sourceMap || !sourceMap.origin.includes('contentful.com')) { - debug.warn; + debug.warn( + "Element has missing or invalid ContentSourceMap, please check if you have correctly enabled ContentSourceMaps and that the element's data originates from Contentful", + { + node, + sourceMap, + }, + ); continue; } diff --git a/packages/live-preview-sdk/src/liveUpdates.ts b/packages/live-preview-sdk/src/liveUpdates.ts index f96247ce..0bb75da0 100644 --- a/packages/live-preview-sdk/src/liveUpdates.ts +++ b/packages/live-preview-sdk/src/liveUpdates.ts @@ -34,6 +34,17 @@ export class LiveUpdates { const subscription = this.subscriptions.get(subscriptionId); + if (!data.sys || !data.sys.id) { + debug.error( + 'Received an update with missing `sys.id`, please provide `sys.id` in order to enable live updates', + { + data, + subscriptionId, + }, + ); + return; + } + if (subscription) { subscription.callback(data); subscription.data = data; diff --git a/packages/live-preview-sdk/src/react.tsx b/packages/live-preview-sdk/src/react.tsx index 046deae5..f5c04914 100644 --- a/packages/live-preview-sdk/src/react.tsx +++ b/packages/live-preview-sdk/src/react.tsx @@ -62,7 +62,7 @@ export function ContentfulLivePreviewProvider({ }: PropsWithChildren): ReactElement { if (!locale) { throw new Error( - 'ContentfulLivePreviewProvider have to be called with a locale property (for example: `{children}`', + 'ContentfulLivePreviewProvider has to be called with a locale property (for example: `{children}`', ); } From b8c3b7f93acc75d8fface75a1d8fc5f8e8d5b47f Mon Sep 17 00:00:00 2001 From: MayaGillilan <101652314+MayaGillilan@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:32:35 +0200 Subject: [PATCH 2/3] chore: cleanup --- packages/live-preview-sdk/src/__tests__/index.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/live-preview-sdk/src/__tests__/index.spec.ts b/packages/live-preview-sdk/src/__tests__/index.spec.ts index 219196de..50356127 100644 --- a/packages/live-preview-sdk/src/__tests__/index.spec.ts +++ b/packages/live-preview-sdk/src/__tests__/index.spec.ts @@ -57,7 +57,7 @@ describe('ContentfulLivePreview', () => { describe('init', () => { describe('should bind the message listeners', () => { - it.only('provide the data to InspectorMode and LiveUpdates', () => { + it('provide the data to InspectorMode and LiveUpdates', () => { const data = { source: LIVE_PREVIEW_EDITOR_SOURCE, value: 'any' }; window.dispatchEvent(new MessageEvent('message', { data })); From fe7f470b5d5bc718a5470d365907fc55c61e6bc5 Mon Sep 17 00:00:00 2001 From: MayaGillilan <101652314+MayaGillilan@users.noreply.github.com> Date: Wed, 25 Sep 2024 10:13:13 +0200 Subject: [PATCH 3/3] refactor: improve messaging --- packages/live-preview-sdk/src/index.ts | 3 ++- packages/live-preview-sdk/src/inspectorMode/utils.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/live-preview-sdk/src/index.ts b/packages/live-preview-sdk/src/index.ts index 00799b90..4c2feb3b 100644 --- a/packages/live-preview-sdk/src/index.ts +++ b/packages/live-preview-sdk/src/index.ts @@ -345,6 +345,7 @@ export class ContentfulLivePreview { return this.liveUpdatesEnabled; } + //TODO: Rename to openEntityInEditor, as assets can also be opened. Would be breaking change static openEntryInEditor(props: LivePreviewProps): void { const defaultProps = { locale: this.locale, @@ -374,7 +375,7 @@ export class ContentfulLivePreview { return; } - debug.error('Please provide field id and entry/asset id to openEntryInEditor.'); + debug.error('Please provide field id and entry/asset id to openEntryInEditor.', { ...props }); } /** diff --git a/packages/live-preview-sdk/src/inspectorMode/utils.ts b/packages/live-preview-sdk/src/inspectorMode/utils.ts index cc448dd1..aa1ce065 100644 --- a/packages/live-preview-sdk/src/inspectorMode/utils.ts +++ b/packages/live-preview-sdk/src/inspectorMode/utils.ts @@ -68,7 +68,7 @@ export function getManualInspectorModeAttributes( }; if (!sharedProps.fieldId) { - debug.warn('Tagged element is missing field ID attribute and cannot be tagged', { + debug.warn('Element is missing field ID attribute and cannot be tagged', { id: element.getAttribute(InspectorModeDataAttributes.ENTRY_ID) ?? element.getAttribute(InspectorModeDataAttributes.ASSET_ID),