Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Try to assert value of the '<property_name>' inside of the value function' error while passing children their properties #273

Closed
gorkat opened this issue Sep 12, 2024 · 5 comments
Labels
bug properties Applies to the property definition render Applies to render and content properties defined as functions

Comments

@gorkat
Copy link

gorkat commented Sep 12, 2024

Hello!

We've been using Hybrids for a while now (and we love it), and it appears that in the latest versions (starting from 9.1.2), passing properties to hybrids children custom elements inside another hybrids custom element's render function makes the component crash with a Try to assert value of the '<property_name>' inside of the value function error.

After a bit of research, the error we get seems related to fix(cache): throw an error when assert while getting the value.

In some of our components, we have hybrids custom elements inside some other hybrids custom elements:

export const renderLinkPopover = (
    host: InternalLinkPopoverElement,
    gettext_provider: GetText,
): UpdateFunction<InternalLinkPopoverElement> =>
    html`
        <section data-role="popover" class="tlp-popover">
            <div class="tlp-popover-arrow"></div>
            <div class="tlp-popover-body">
                <div class="tlp-button-bar">
                    <open-link-button
                        gettext_provider="${gettext_provider}">
                        sanitized_link_href="${host.sanitized_link_href}"
                    </open-link-button>
                    <copy-to-clipboard-button
                        gettext_provider="${gettext_provider}"
                        value_to_copy="${host.sanitized_link_href}"
                    ></copy-to-clipboard-button>
                </div>
        </section>
    `.style(popover_styles);

This render function works fine with [email protected].

This error prevents us to upgrade to the latest version (9.1.5 at the time of writing).

We couldn't find any documentation about it, and there is not that much of context around the patch mentioned above.

What is the right way to pass children custom elements their properties ? What are we doing wrong?

Best regards!

@smalluban
Copy link
Contributor

smalluban commented Sep 12, 2024

Hi. Thanks for raising this up. However, I cannot reproduce the problem. Here you have a test, which shows that everything works as expected: #274

The template engine returns an update function when called, it makes assertions, but the function is not called when the render evaluates. It's called in the observe callback, outside of that context.

If you have that error, it means, that you must directly set a property value inside of that get call of render or other property of the parent element.

Can you provide a call stack of that error? Or simplify the example, still keeping the error. I think it might be helpful, to show where exactly the assertion is made.

@smalluban smalluban added render Applies to render and content properties defined as functions properties Applies to the property definition labels Sep 12, 2024
@gorkat
Copy link
Author

gorkat commented Sep 12, 2024

Sure !

I'll try to provide as much information as I can.

Considering the previous example with [email protected], I get the following stack trace (in the web console):

Error while updating template expression in <tuleap-prose-mirror-link-popover-element>:
| <section data-role="popover" class="tlp-popover prose-mirror-editor-popover-links">
|   <div class="tlp-popover-arrow"></div>
|   <div class="tlp-popover-body">
|       <div class="tlp-button-bar">
|           <open-link-button
|               gettext_provider="${...}">
----------------------------------^^^^^^
|               sanitized_link_href="${...}"
|           </open-link-button>
|           <copy-to-clipboard-button
|               gettext_provider="${...}"
|               value_to_copy="${...}"
|           ></copy-to-clipboard-button>
|       </div>
| </section>

// -------------------------------------

Error: Try to assert value of the 'gettext_provider' inside of the value function
    assert https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21013
    assert$1 https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21250
    resolveProperty https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21883
    updateTemplateInstance https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:22278
    fn https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:22461
    value https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21126
    value https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:22669
    get$1 https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:20996
    get2 https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21247
    connect https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:22652
    1 https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21257
    connectedCallback https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21183
    execute https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:20923
    promise callback*add https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:20913
    connectedCallback https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:21216
    insertLinkPopover https://tuleap-web.tuleap-aio-dev.docker/assets/artidoc/artidoc/assets/SectionDescriptionEditorProseMirror--iFYM5xn.js:22702
    [...] // unrelevant stuff truncated

Now regarding the components, the parent element's definition looks like this:

define<InternalLinkPopoverElement>({
    tag: TAG,
    gettext_provider: (host, gettext_provider) => gettext_provider,
    sanitized_link_href: (host: InternalLinkPopoverElement): string => sanitizeURL(host.link_href),
    link_href: (host, link_href) => link_href,
    // [...]
    render: {
        shadow: false,
        value: (host) => renderLinkPopover(host, host.gettext_provider),
    },
});

The <open-link-button/> definition looks like this:

define<InternalOpenLinkButtonElement>({
    tag: TAG,
    gettext_provider: (host, gettext_provider) => gettext_provider,
    sanitized_link_href: "",
    render: (host) => renderOpenLinkButton(host, host.gettext_provider),
});

and its render function:

export const renderOpenLinkButton = (
    host: InternalOpenLinkButtonElement,
    gettext_provider: GetText,
): UpdateFunction<InternalOpenLinkButtonElement> => html`
    <div class="tlp-button-bar-item">
        <a
            class="tlp-button-outline tlp-button-secondary tlp-button-small"
            title="${gettext_provider.gettext("Open link")}"
            href="${host.sanitized_link_href}"
            target="_blank"
            data-test="open-link-button"
        >
            <i class="fa-solid fa-external-link-alt" role="img"></i>
        </a>
    </div>
`;

Thank you for your time, I hope it helps :)

@smalluban
Copy link
Contributor

Thanks for the details. I know how to fix it, still not removing the constraint completely, but I would like to understand why it happens first.

I'll look at this on Monday or later. Until I fix it, please keep the latest version without the bug.

@smalluban smalluban added the bug label Sep 13, 2024
@smalluban
Copy link
Contributor

smalluban commented Sep 20, 2024

After a while, I found a case, where it throws - the render() must be called manually in another property - then the update process is made inside of the get call of that property. The fix is merged. I'll release a new version in a few minutes.

@gorkat
Copy link
Author

gorkat commented Sep 20, 2024

Hello,

We have successfully upgraded to [email protected] without any problem 🎉

Thank you for the fix!

LeSuisse pushed a commit to Enalean/tuleap that referenced this issue Sep 23, 2024
part of story #38627 : Choice of a new Rich Text Editor

Currently, the design of the popover buttons inclusion is not really
good:
- Each button element must extend a "type" property, but needs to
omit it in their component definition (in order to not have to define a
readonly property).
- We need a button-creator to render each button. It has to somehow pass
  the props to the buttons so they can render themselves.

In short, this is kind of a mess.

We had to establish this strategy because there were a bug in hybrids
preventing us to pass children components their properties ([see GH
issue](hybridsjs/hybrids#273)).

Now it has been corrected, and that hybrids has been successfully upgraded to version 9.1.6 in the whole codebase, then we can refactor the current popover buttons inclusion system to make it simpler
and more convenient to use.

-- no functional change expected

Change-Id: Iff689add5532b1d0ea6a5ae15f2b7218402d5e4f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug properties Applies to the property definition render Applies to render and content properties defined as functions
Development

No branches or pull requests

2 participants