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

Components added with ComponentRenderer can cause client-side errors [2 days] #4554

Closed
tomivirkki opened this issue Jan 18, 2023 · 2 comments
Assignees
Labels
bug Something isn't working needs research

Comments

@tomivirkki
Copy link
Member

tomivirkki commented Jan 18, 2023

Description

The ComponentRenderer, uses a parent container for all of the rendered Component instances that get passed to the client. The container gets added as a virtual child to the owner component so it doesn't automatically appear in the DOM on the browser. The individual child components originally included in the container get picked to the DOM on demand, for example, to the Grid column cells, when you scroll the Grid.

Some Flow components add internal JS executions that access the Web Component public API and Flow runs the executions for virtual child components even if they haven't actually been placed in the DOM. This is typically fine. However, in certain cases, accessing the Web Component public API may internally end up accessing the shadow DOM, etc, but since the element isn't actually in the DOM yet, the shadow DOM / internal props such as this.$ may have not yet been initialized and an error gets thrown.

It's debatable whether this should be fixed in the Web Components so that, for example, calling dangerouslySetHtmlValue for an unattached <vaadin-rich-text-editor> wouldn't throw an exception:

const rte = document.createElement('vaadin-rich-text-editor');
rte.dangerouslySetHtmlValue('<p>Some <b>bold</b> text</p>');
document.body.appendChild(rte);

The issue could also be fixed by appending the container to the owner as a regular child instead of a virtual child. As a consequence, there would be an additional hidden element in the DOM of the ComponentRenderer owners:

<vaadin-grid-column>
  <div class="component-renderer-container" hidden></div>
</vaadin-grid-column>

<vaadin-combo-box>
  <div class="component-renderer-container" hidden></div>
</vaadin-combo-box>

<vaadin-virtual-list>
  <div class="component-renderer-container" hidden></div>
</vaadin-virtual-list>

The container can be removed from the DOM programmatically in case it's considered problematic to have it there

Related issues:

Expected outcome

Client-side exceptions shouldn't occur.

Minimal reproducible example

See #4551

Steps to reproduce

See #4551

Environment

Vaadin version(s): 23+ (probably also V14)

Browsers

Issue is not browser related

@tomivirkki tomivirkki added the bug Something isn't working label Jan 18, 2023
@yuriy-fix yuriy-fix changed the title Components added with ComponentRenderer can cause client-side errors Components added with ComponentRenderer can cause client-side errors [2 days] Jan 18, 2023
@yuriy-fix
Copy link
Contributor

yuriy-fix commented Jan 18, 2023

Let's check how much effort would be needed for this task including testing it with the components. (within timebox of 2 days)

@tomivirkki
Copy link
Member Author

It's debatable whether this should be fixed in the Web Components so that, for example, calling dangerouslySetHtmlValue for an unattached wouldn't throw an exception

It's better to fix the possible issues in the web components (for example vaadin/web-components#6871) instead of changing how the ComponentRenderer works, which would be more like a workaround for the root cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs research
Projects
None yet
Development

No branches or pull requests

2 participants