fix(cms-base): improve getOptionsFromNode helper (#542)

.changeset/swift-flies-fetch.md

@@ -0,0 +1,5 @@
+---
+"@shopware-pwa/cms-base": patch
+---
+
+Potential problems with CmsElementText rendering, when Node object is incorrect

packages/cms-base/components/public/cms/element/CmsElementText.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import type { CmsElementText } from "@shopware-pwa/composables-next";
-import { useCmsElementConfig } from "#imports";
+import { useCmsElementConfig, useUrlResolver } from "#imports";
 import { computed, getCurrentInstance, h } from "vue";
 import { decodeHTML } from "entities";
 import type { CSSProperties } from "vue";
@@ -25,6 +25,8 @@ const style = computed<CSSProperties>(() => ({
 const hasVerticalAlignment = computed(() => !!style.value.alignItems);
 
 const CmsTextRender = () => {
+  const { resolveUrl } = useUrlResolver();
+
   const config = {
     textTransformer: (text: string) => decodeHTML(text),
     extraComponentsMap: {
@@ -42,7 +44,7 @@ const CmsTextRender = () => {
             {
               class:
                 "underline text-base font-normal text-primary hover:text-secondary-900",
-              ...getOptionsFromNode(node).attrs,
+              ...getOptionsFromNode(node, resolveUrl).attrs,
             },
             [...children],
           );
@@ -71,7 +73,7 @@ const CmsTextRender = () => {
             "a",
             {
               class: _class,
-              ...getOptionsFromNode(node).attrs,
+              ...getOptionsFromNode(node, resolveUrl).attrs,
             },
             [...children],
           );
@@ -95,7 +97,7 @@ const CmsTextRender = () => {
             "span",
             {
               style: newStyle,
-              ...getOptionsFromNode(node).attrs,
+              ...getOptionsFromNode(node, resolveUrl).attrs,
             },
             [...children],
           );
@@ -106,7 +108,10 @@ const CmsTextRender = () => {
           return node.type === "tag" && node.name === "img";
         },
         renderer(node: any, children: any, createElement: any) {
-          return createElement("img", getOptionsFromNode(node)?.attrs);
+          return createElement(
+            "img",
+            getOptionsFromNode(node, resolveUrl)?.attrs,
+          );
         },
       },
     },
@@ -115,7 +120,7 @@ const CmsTextRender = () => {
     mappedContent.value?.length > 0
       ? mappedContent.value
       : "<div class='cms-element-text missing-content-element'></div>";
-  return renderHtml(rawHtml, config, h, context);
+  return renderHtml(rawHtml, config, h, context, resolveUrl);
 };
 </script>
 <template>

packages/cms-base/helpers/html-to-vue/getOptionsFromNode.test.ts

@@ -0,0 +1,107 @@
+import { getOptionsFromNode } from "./getOptionsFromNode";
+import { describe, expect, it, vi, beforeEach } from "vitest";
+
+describe("getOptionsFromNode", () => {
+  const consoleErrorSpy = vi.spyOn(console, "error");
+  const urlResolverMock = vi.fn();
+
+  beforeEach(() => {
+    vi.resetAllMocks();
+    consoleErrorSpy.mockImplementation(() => {});
+    urlResolverMock.mockImplementation((url) => "resolved-url" + url);
+  });
+
+  it("should return empty object if node is undefined", () => {
+    const options = getOptionsFromNode(undefined, urlResolverMock);
+
+    expect(options).toEqual({
+      attrs: {},
+    });
+  });
+
+  it("should return options object with style, classNames, and align", () => {
+    const node = {
+      attrs: {
+        style: "color: red",
+        class: "my-class",
+        align: "center",
+      },
+    };
+
+    const options = getOptionsFromNode(node, urlResolverMock);
+
+    expect(options).toEqual({
+      style: "color: red",
+      class: "my-class",
+      align: "center",
+      attrs: {},
+    });
+  });
+
+  it("should return options object without style, classNames, and align if they are not present in node.attrs", () => {
+    const node = {
+      attrs: {},
+    };
+
+    const options = getOptionsFromNode(node, urlResolverMock);
+
+    expect(options).toEqual({
+      attrs: {},
+    });
+  });
+
+  it("should return empty object when no attrs in node", () => {
+    const node = {
+      attrs: undefined,
+    };
+
+    const options = getOptionsFromNode(node, urlResolverMock);
+
+    expect(options).toEqual({
+      attrs: {},
+    });
+  });
+
+  it("should resolve URL if attrs.href exists", () => {
+    const node = {
+      attrs: {
+        href: "/path/to/page",
+      },
+    };
+
+    const options = getOptionsFromNode(node, urlResolverMock);
+
+    expect(options.attrs.href).toEqual("resolved-url/path/to/page");
+  });
+
+  it('should add additional attrs to "attrs" object', () => {
+    const node = {
+      attrs: {
+        href: "/path/to/page",
+        "data-test": "test",
+      },
+    };
+
+    const options = getOptionsFromNode(node, urlResolverMock);
+
+    expect(options.attrs.href).toEqual("resolved-url/path/to/page");
+    expect(options.attrs["data-test"]).toEqual("test");
+  });
+
+  it("should show console error if something went wrong", () => {
+    const node = {
+      attrs: {
+        href: "/path/to/page",
+      },
+    };
+
+    const consoleErrorSpy = vi.spyOn(console, "error");
+
+    const options = getOptionsFromNode(node, undefined as any);
+
+    expect(options).toEqual({
+      attrs: {},
+    });
+    expect(consoleErrorSpy).toBeCalled();
+  });
+});

packages/cms-base/helpers/html-to-vue/getOptionsFromNode.ts

@@ -1,5 +1,3 @@
-import { useUrlResolver } from "#imports";
-
 export type NodeObject = {
   type: string;
   name: string;
@@ -11,43 +9,44 @@ export type NodeObject = {
 
 type Options = {
   align?: string;
-  attrs?: Record<string, string>;
+  attrs: Record<string, string>;
   class?: string;
   color?: string;
   style?: string;
+  href?: string;
 };
 
-export function getOptionsFromNode(node: any): Options {
-  let style = undefined;
-  let classNames = undefined;
-  let align = undefined;
-
-  if (node.attrs.style && node.attrs.style !== "") {
-    style = node.attrs.style;
-    delete node.attrs.style; // we delete the nodes otherwise it would be added to rest again
-  }
-
-  if (node.attrs.class && node.attrs.class !== "") {
-    classNames = node.attrs.class;
-    delete node.attrs.class;
-  }
-
-  if (node.attrs.align && node.attrs.align !== "") {
-    align = node.attrs.align;
-    delete node.attrs.align;
-  }
-  const attrs = Object.keys(node.attrs).length === 0 ? null : { ...node.attrs };
-
-  // Resolve URL if exist
-  if (attrs?.href) {
-    const { resolveUrl } = useUrlResolver();
-    attrs.href = `${resolveUrl(attrs.href)}`;
-  }
-
-  return {
-    ...(typeof align != "undefined" && { align }),
-    ...(typeof attrs != "undefined" && { attrs }),
-    ...(typeof classNames != "undefined" && { class: classNames }),
-    ...(typeof style != "undefined" && { style }),
+export function getOptionsFromNode(
+  node: any,
+  resolveUrl: (url: string) => string,
+): Options {
+  const response: Options = {
+    attrs: {},
   };
+  try {
+    if (!node?.attrs) {
+      return response;
+    }
+
+    const { align, style, class: classNames, href, ...attrs } = node.attrs;
+
+    if (align) {
+      response.align = align;
+    }
+    if (style) {
+      response.style = style;
+    }
+    if (classNames) {
+      response.class = classNames;
+    }
+    if (attrs && Object.keys(attrs).length > 0) {
+      response.attrs = attrs;
+    }
+    if (href) {
+      response.attrs.href = resolveUrl(href);
+    }
+  } catch (e) {
+    console.error("[Shopware][cms][getOptionsFromNode] error", e);
+  }
+  return response;
 }

packages/cms-base/helpers/html-to-vue/renderToHtml.ts

@@ -28,10 +28,17 @@ export function renderHtml(
   config: Partial<DefaultConfig>,
   createElement: any,
   context: any,
+  resolveUrl: (url: string) => string,
 ) {
   const mergedConfig = Object.assign(defaultConfig, config);
   const _ast = generateAST(html);
   const _rectifiedAst = rectifyAST(_ast, config);
 
-  return renderer(_rectifiedAst, mergedConfig, createElement, context);
+  return renderer(
+    _rectifiedAst,
+    mergedConfig,
+    createElement,
+    context,
+    resolveUrl,
+  );
 }

packages/cms-base/helpers/html-to-vue/renderer.ts

@@ -13,7 +13,7 @@ import { getOptionsFromNode } from "./getOptionsFromNode";
  * @param {*} createElement vue's createElement
  * @param {*} context vue functional component context
  */
-export function renderer(ast, config, createElement, context) {
+export function renderer(ast, config, createElement, context, resolveUrl) {
   function _render(h, node, parent, key, index) {
     if (Array.isArray(node)) {
       const nodes = [];
@@ -28,7 +28,7 @@ export function renderer(ast, config, createElement, context) {
         return config.textTransformer(node.content); // return text
       }
       if (node.type === "tag") {
-        const transformedNode = getOptionsFromNode(node);
+        const transformedNode = getOptionsFromNode(node, resolveUrl);
         const children = [];
         node.children.forEach((child, index) => {
           children.push(_render.call(this, h, child, transformedNode, index));

packages/cms-base/package.json

@@ -32,7 +32,8 @@
     "eslint": "eslint components/**/*.vue* --fix --max-warnings=0",
     "lint": "pnpm run eslint && pnpm run typecheck",
     "typecheck": "tsc --noEmit",
-    "test": "echo \"Warn: no test specified yet\""
+    "test": "vitest run",
+    "test:watch": "vitest"
   },
   "dependencies": {
     "@nuxt/kit": "^3.9.3",
@@ -49,13 +50,15 @@
   "devDependencies": {
     "@nuxt/schema": "^3.9.3",
     "@shopware-pwa/types": "workspace:*",
+    "@vitest/coverage-v8": "^1.2.1",
     "@vue/eslint-config-typescript": "^12.0.0",
     "eslint-config-shopware": "workspace:*",
     "eslint-plugin-vue": "^9.20.1",
     "nuxt": "^3.9.3",
     "tsconfig": "workspace:*",
     "typescript": "^5.3.3",
     "unbuild": "^2.0.0",
+    "vitest": "^1.2.1",
     "vue-eslint-parser": "^9.4.2",
     "vue-router": "^4.2.5",
     "vue-tsc": "^1.8.27"

packages/cms-base/vitest.config.ts

@@ -0,0 +1,15 @@
+import { defineConfig } from "vitest/config";
+import { resolve } from "path";
+
+export default defineConfig({
+  test: {
+    environment: "happy-dom",
+    coverage: {
+      enabled: true,
+    },
+    alias: {
+      "#imports": resolve(__dirname, "./composables.d.ts"),
+      "#shopware": resolve(__dirname, "./types/api-types.d.ts"),
+    },
+  },
+});