bigcommerce / paper-handlebars

Paper plugin for rendering via Handlebars.js
BSD 4-Clause "Original" or "Old" License
12 stars 36 forks source link

dynamicComponent missing context / handlebars dynamic partials broken #133

Open peterrakmanyi opened 3 years ago

peterrakmanyi commented 3 years ago

I've just added a new theme setting to switch the way swatch options are rendered. It works fine, but the theme_settings object is not available in the context of the component.

This would normally be fixed by adding it as an input for the handlebars component and it works as expected.

{{> components/products/options/swatch theme_settings=../theme_settings}}

But, that is not how components are used for product options. The dynamicComponent custom helper decides which component to use, but does not add the inputs. This doesn't work:

{{{dynamicComponent 'components/products/options' theme_settings=../theme_settings}}}

What do you think about this?

peterrakmanyi commented 3 years ago

Dynamic partials also don't work:

{{> (concat "components/products/options/" this.partial) theme_settings=../theme_settings }}

results in: The partial components/products/options/set-rectangle could not be found

mattcoy-arcticleaf commented 3 years ago

I don't think dynamicComponent is set up to pull in context. See my response on this thread here: https://support.bigcommerce.com/s/question/0D54O00006n4TTLSA2/possible-to-pass-a-parameter-to-the-product-options-dynamic-component

peterrakmanyi commented 3 years ago

@mattcoy-arcticleaf Yes, that is exactly the problem. The dynamic component is so you don't have to specify the type of the option, so you can avoid an ugly switch statement to include all possible option types. But without the context or even custom variables it is only good for a very narrow set of use cases and defeats the purpose of this helper. You get far more compatibility with a switch statement like this:

{{#each product.options}}
    {{#eq this.partial "date"}}{{>components/products/options/date theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "input-checkbox"}}{{>components/products/options/input-checkbox theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "input-file"}}{{>components/products/options/input-file theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "input-numbers"}}{{>components/products/options/input-numbers theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "input-text"}}{{>components/products/options/input-text theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "product-list"}}{{>components/products/options/product-list theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "set-radio"}}{{>components/products/options/set-radio theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "set-rectangle"}}{{>components/products/options/set-rectangle theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "set-select"}}{{>components/products/options/set-select theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "swatch"}}{{>components/products/options/swatch theme_settings=../theme_settings}}{{/eq}}
    {{#eq this.partial "textarea"}}{{>components/products/options/textarea theme_settings=../theme_settings}}{{/eq}}
{{/each}}

This could be one line if the dynamic component feature was fully implemented, but it is not. I consider this a bug.

LordZardeck commented 2 years ago

This is also a huge issue for me, again same use case. I need to access theme settings within the dynamic component, but I am unable to. Ideally I feel like you should be able to use the standard @root keyword. Not being able to access any context outside the component is a bug IMO, but I'd even take being able to pass something in. It's been 10 months since this ticket is opened, and no response. Is there any chance of this being resolved?

LordZardeck commented 2 years ago

Actually, I'm not sure I understand why dynamicComponent was ever implemented. Handlebars supports dynamic partials natively. From what I can tell, the only reason this isn't working is because of something complicated the stencil server renderer is doing for determining what templates to load, which it isn't able to determine with it's simple regex search. I'm not sure why the stencil server is doing this in the first place, but it would be nice if it can't simply have all templates available, if it could lazy-load them.

LordZardeck commented 2 years ago

Specifically this: https://github.com/bigcommerce/stencil-cli/blob/master/lib/template-assembler.js#L98 is the problem. Since it can't detect usages of native dynamic partials, it's not loading any at all.

LordZardeck commented 2 years ago

@peterrakmanyi Here's a workaround for anyone wanting to use native dynamic components. Since the only thing preventing the native one working is the server not knowing how to load the templates, you just need to trick it into loading it the only way it knows how: the dynamic component helper

{{#if false}}{{ dynamicComponent 'components/products/options'}}{{/if}}
{{#each product.options}}
    {{> (concat 'components/products/options/' this.partial) theme_settings=../theme_settings}}
{{/each}}

By making sure we wrap it in a conditional that never renders, we essentially just tricked it into loading all templates within components/products/options, and then the native dynamic partial functionality has the template to load, allowing us to pass in properties like normal. Hopefully this proves helpful for people.

woodbrettm commented 2 years ago

@LordZardeck I'm getting the not found errors as well. When using the native dynamic partial syntax

{{> (concat 'components/custom/webpage-content/page-' page.id)}}

it'll say it's not found, even though I confirmed it matched completely. I also tried with your method with the if block and dynamic component.

Even if I hardcode page.id as a string (e.g. '91'). What's odd is if I create a template called default.html and put that in the folder, then reference it:

{{> (concat 'components/custom/webpage-content/default' '')}}

works fine, which I find super strange. Change the name to anything else? Nope, even if the correct template is in the folder haha..

Below works for me so far, but it's not perfect as you have to manually reconstruct the this/context object with keys you want to pass, but at least has the flexibility of passing in extra keys like partial, and any others you want.

In this example in the page.html template, I don't want to create separate page templates atm, so this helps me render different page content based on the id

{{#any
    (if page.id '===' 91)
    (if page.id '===' 94)
}}

    {{#withHash
        partial=(add 'page-' page.id)
        page=page
        pages=pages
        urls=urls
    }}

        {{{dynamicComponent 'components/custom/webpage-content'}}}

    {{/withHash}}

{{else}}

    {{> components/custom/webpage-content/default}}

{{/any}}

{{> layout/base}}
woodbrettm commented 2 years ago

Didn't know {{#with}} is a thing, so you could do the following:

{{#withHash
    partial=(add 'page-' page.id)
}}

    {{#with (extend this ../this)}}
        {{{dynamicComponent 'components/custom/webpage-content'}}}
    {{/with}}

{{/withHash}}