Skip to content

Commit

Permalink
fix: improve error messaging (#893)
Browse files Browse the repository at this point in the history
* feat: improve error messaging

* chore: cleanup

* refactor: improve messaging
  • Loading branch information
MayaGillilan authored Sep 25, 2024
1 parent 16d5837 commit 5b87339
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 14 deletions.
2 changes: 1 addition & 1 deletion packages/live-preview-sdk/src/__tests__/init.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'})`)",
);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/live-preview-sdk/src/__tests__/react.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('ContentfulLivePreviewProvider', () => {
// @ts-expect-error -- case locale not provided (e.g. JavaScript usage)
() => render(<ContentfulLivePreviewProvider>Hello World</ContentfulLivePreviewProvider>),
).toThrowError(
'ContentfulLivePreviewProvider have to be called with a locale property (for example: `<ContentfulLivePreviewProvider locale="en-US">{children}</ContentfulLivePreviewProvider>`',
'ContentfulLivePreviewProvider has to be called with a locale property (for example: `<ContentfulLivePreviewProvider locale="en-US">{children}</ContentfulLivePreviewProvider>`',
);

spy.mockRestore();
Expand Down
5 changes: 3 additions & 2 deletions packages/live-preview-sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class ContentfulLivePreview {
static init(config: ContentfulLivePreviewInitConfig): Promise<InspectorMode | null> | 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'})`)",
);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -374,7 +375,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.', { ...props });
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -174,11 +175,15 @@ describe('getAllTaggedElements', () => {
`<span>${combine('Test', createSourceMapFixture('ignore_origin', { origin: 'example.com' }))}</span>`,
);

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', () => {
Expand Down Expand Up @@ -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);
Expand Down
22 changes: 18 additions & 4 deletions packages/live-preview-sdk/src/inspectorMode/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ export function getManualInspectorModeAttributes(
manuallyTagged: true,
};

if (!sharedProps.fieldId) {
debug.warn('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 };
Expand Down Expand Up @@ -158,18 +168,16 @@ function hasTaggedParent(
export function getAllTaggedElements({
root = window.document,
options,
ignoreManual,
}: {
root?: any;
options: Omit<InspectorModeOptions, 'targetOrigin'>;
ignoreManual?: boolean;
}): {
taggedElements: PrecaulculatedTaggedElement[];
manuallyTaggedCount: number;
automaticallyTaggedCount: number;
autoTaggedElements: AutoTaggedElement<Element>[];
} {
const alreadyTagged = ignoreManual
const alreadyTagged = options.ignoreManuallyTaggedElements
? []
: root.querySelectorAll(
`[${InspectorModeDataAttributes.ASSET_ID}][${InspectorModeDataAttributes.FIELD_ID}], [${InspectorModeDataAttributes.ENTRY_ID}][${InspectorModeDataAttributes.FIELD_ID}]`,
Expand All @@ -190,7 +198,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;
}

Expand Down
11 changes: 11 additions & 0 deletions packages/live-preview-sdk/src/liveUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/live-preview-sdk/src/react.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export function ContentfulLivePreviewProvider({
}: PropsWithChildren<ContentfulLivePreviewInitConfig>): ReactElement {
if (!locale) {
throw new Error(
'ContentfulLivePreviewProvider have to be called with a locale property (for example: `<ContentfulLivePreviewProvider locale="en-US">{children}</ContentfulLivePreviewProvider>`',
'ContentfulLivePreviewProvider has to be called with a locale property (for example: `<ContentfulLivePreviewProvider locale="en-US">{children}</ContentfulLivePreviewProvider>`',
);
}

Expand Down

0 comments on commit 5b87339

Please sign in to comment.