Passing `Data` to components breaks their behavior

[naiyerasif]

Version

1.15.1

Platform

Windows

What steps will reproduce the bug?

Currently, Lume injects the components in the templates so that they are easily accessible by the layouts and templates. For example, I have an _components/icon.ts as follows.

import { attributes } from "lume/plugins/attributes.ts"

const defaults = { class: ["icon"], role: "img" }

export default function (name: string, { title, ...attribs }: { title?: string; [key: string]: unknown }) {
  const href = `#${name}`
  const props = attributes({ ...attribs, ...defaults })

  if (title) {
    return `
      <svg aria-labelledby="${title}" ${props}>
        <title id="${title}">${title}</title>
        <use href="${href}"/>
      </svg>
    `
  } else {
    return `
      <svg aria-hidden="true" ${props}>
        <use href="${href}"/>
      </svg>
    `
  }
}

Lume fails to render the component when I try to use it in a template through comp as described by the docs, for example, like this.

export const title = "Home"
export const renderOrder = 1

export default function ({ comp }) {
  const icon = comp.icon("moon", { slot: "dark" })
  console.log(icon)
  return `<h1>${title}</h1>${icon}`
}

The issue originates because of the ...attribs specified in the component as I sometimes expect dynamic HTML attributes to be passed to the component (for example, slot in this case). Unfortunately, Lume also passes the entire Data object to the component and the attributes plugin fails to handle it.

This also leads to unexpected issues. For example, sometimes, I may pass a title attribute to the component. But when I export a title in the template, that's also being implicitly passed to the component which is definitely not correct.

How often does it reproduce? Is there a required condition?

Reproducing this does not require anything specific. Here's the repository with the reproduction: https://github.com/naiyerasif/lume-components-issue

Launch the application through deno task serve and you should encounter the issue described above.

What is the expected behavior?

The components should receive only those arguments which are explicitly defined in their contract. The Data object should not be passed to them at all.

What do you see instead?

I get the following error on console.

Error: Error rendering this page
- page: /index.tmpl.ts
    at (https://deno.land/x/lume@v1.15.1/core/renderer.ts:138:19)
    at async concurrent (https://deno.land/x/lume@v1.15.1/core/utils.ts:193:3)
    at async Renderer.renderPages (https://deno.land/x/lume@v1.15.1/core/renderer.ts:126:7)
    at async Site.#buildPages (https://deno.land/x/lume@v1.15.1/core/site.ts:590:5)
    at async Site.build (https://deno.land/x/lume@v1.15.1/core/site.ts:518:9)
    at async build (https://deno.land/x/lume@v1.15.1/cli/build.ts:36:3)
    at async Command.execute (https://deno.land/x/cliffy@v0.25.7/command/command.ts:1794:7)
    at async Command.parseCommand (https://deno.land/x/cliffy@v0.25.7/command/command.ts:1639:14)
    at async (https://deno.land/x/lume@v1.15.1/cli.ts:158:3)

Caused by TypeError: value.replace is not a function
    at escape (https://deno.land/x/lume@v1.15.1/plugins/attributes.ts:124:16)
    at joinAttributes (https://deno.land/x/lume@v1.15.1/plugins/attributes.ts:117:29)
    at attributes (https://deno.land/x/lume@v1.15.1/plugins/attributes.ts:27:10)
    at default (file:///E:/web/lume-components-issue/src/_components/icon.ts:7:16)
    | 6 |       const href = `#${name}`
    | 7 |       const props = attributes({ ...attribs, ...defaults })
    | ~ | ~~~~~~~~~~~~~~~^
    | 8 |

    at ModuleEngine.renderSync (https://deno.land/x/lume@v1.15.1/plugins/modules.ts:38:9)
    at (https://deno.land/x/lume@v1.15.1/core/component_loader.ts:100:20)
    at Object.render (https://deno.land/x/lume@v1.15.1/core/component_loader.ts:98:32)
    at Proxy.<anonymous> (https://deno.land/x/lume@v1.15.1/core/source.ts:542:60)
    at default (file:///E:/web/lume-components-issue/src/index.tmpl.ts:5:20)
    | 4 | export default function ({ comp }) {
    | 5 |       const icon = comp.icon("moon", { slot: "dark" })
    | ~ | ~~~~~~~~~~~~~~~~~~~^
    | 6 |       console.log(icon)

Additional information

No response

[naiyerasif]

As a workaround, I am explicitly importing the components as follows.

import icon from "./_components/icon.ts"
export const title = "Home"
export const renderOrder = 1

export default function () {
  return `<h1>${title}</h1>${icon("moon", { slot: "dark" })}`
}

[oscarotero]

Mmm, ok. Interesting use case.

Components are designed to work with any template engine, so they need to receive all properties in the first argument in an object (this is how JSX works). And they also needs access to the comp object in order to create nested components, and other global data like search, etc.

Both requirements are not compatible with the use of spread {...attribs} to collect the remaining attributes. I have to think in a way to fix this. Maybe defining these properties as not enumerable. 🤔

Another workaround is using an attr property to store all attributes:

You component:

export default function ({ name, title, attrs }) {
}

In your template:

const icon = comp.icon({ name: "moon", attrs: {slot: "dark"} })

I'm going to do some tests to see if I can fix it. Thanks.

[oscarotero]

I've tried to make the inherited properties not enumerable but this introduce some issues in nunjucks (that only detect enumerable properties).

So finally I've decided to make it optional. You can configure your components to inherit the context data (by default) or disable it, so the only data received is the directly passed to the component.

In your component, set the property inheritData to false:

export const inheritData = false;

export default function ({name, title, ...attrs}) {
   // code here
}

This is not released yet, but you can use it by upgrading Lume to the latest development version with deno task lume upgrade --dev.

Let me know if this works to you. Thanks!

[naiyerasif]

@oscarotero Thanks for taking a stab at this. I tried the dev build and I am still getting the same error. Here's what I changed: https://github.com/naiyerasif/lume-components-issue/commit/bdbb735693d001618d1c1607f2f7fde8fa5e2c98#diff-ca7f5073a339af20e76bf73a56628c71ae873d2fe29dce97e2677a03ad9dc8bc

[oscarotero]

Components need all properties in the first argument. You have to change the icon to:

export default function ({ name, title, ...attribs }: { name: string, title?: string; [key: string]: unknown }) {
}

(Sorry the example in my previous comment was wrong. I just changed it)

[naiyerasif]

Yup, that worked. Thanks @oscarotero.

[oscarotero]

Lume 1.15.2 was released, including this change.