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

vaadin-grid internal error when rendered in Jest environment causes test suite to fail to run #7370

Open
j-scola opened this issue Apr 30, 2024 · 2 comments
Labels
enhancement New feature or request Impact: Low test Failing or incorrect test vaadin-grid

Comments

@j-scola
Copy link

j-scola commented Apr 30, 2024

Description

My team is using vaadin components in our Lit-Element/TypeScript application. I've written some Jest tests against some components that render instances of vaadin components. In one case, I'm rendering a component in the Jest environment that wraps around a vaadin Grid and I've encountered an error that causes the test suite to fail. I haven't been able to reproduce this error in the browser, only in Jest.

  ● Test suite failed to run

    TypeError: Cannot read properties of undefined (reading 'remove')

      at remove (node_modules/@vaadin/grid/src/vaadin-grid-column-mixin.js:691:21)
          at Array.forEach (<anonymous>)
      at GridColumn.forEach [as __headerFooterPartNameChanged] (node_modules/@vaadin/grid/src/vaadin-grid-column-mixin.js:688:9)
      at Object.apply [as fn] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1038:38)
      at fn (node_modules/@polymer/polymer/lib/mixins/property-effects.js:140:16)
      at GridColumn.runEffects [as _propertiesChanged] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1922:7)
      at GridColumn._propertiesChanged [as _flushProperties] (node_modules/@polymer/polymer/lib/mixins/properties-changed.js:384:14)
      at GridColumn._flushProperties [as _invalidateProperties] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1748:14)
      at GridColumn._invalidateProperties (node_modules/@polymer/polymer/lib/mixins/properties-changed.js:170:18)
      at node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:715:40
          at Array.forEach (<anonymous>)
      at Grid.forEach [as _updateRow] (node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:658:10)
      at _updateRow (node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:880:14)
          at Array.forEach (<anonymous>)
      at forEach (node_modules/@vaadin/grid/src/vaadin-grid-helpers.js:26:27)
      at Grid._renderColumnTree (node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:879:22)
      at Grid._renderColumnTree [as _columnTreeChanged] (node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:822:12)
      at Object.apply [as fn] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1038:38)
      at fn (node_modules/@polymer/polymer/lib/mixins/property-effects.js:140:16)
      at Grid.runEffects [as _propertiesChanged] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1922:7)
      at Grid._propertiesChanged [as _flushProperties] (node_modules/@polymer/polymer/lib/mixins/properties-changed.js:384:14)
      at Grid._flushProperties [as _invalidateProperties] (node_modules/@polymer/polymer/lib/mixins/property-effects.js:1748:14)
      at Grid._invalidateProperties (node_modules/@polymer/polymer/lib/mixins/properties-changed.js:170:18)
      at Grid._updateColumnTree (node_modules/@vaadin/grid/src/vaadin-grid-dynamic-columns-mixin.js:112:25)
      at Debouncer._updateColumnTree [as _callback] (node_modules/@vaadin/grid/src/vaadin-grid-dynamic-columns-mixin.js:94:14)
      at _callback (node_modules/@vaadin/component-base/src/debounce.js:84:12)
      at cb (node_modules/@vaadin/component-base/src/async.js:35:9)
      at _loop (node_modules/@vaadin/component-base/src/async.js:31:31)
      at microtaskFlush (node_modules/@vaadin/component-base/src/async.js:179:28)
      at invokeTheCallbackFunction (node_modules/jsdom/lib/jsdom/living/generated/Function.js:19:26)
      at node_modules/jsdom/lib/jsdom/browser/Window.js:554:9

The method at the point of error: remove (node_modules/@vaadin/grid/src/vaadin-grid-column-mixin.js:691:21) is as follows:

    __headerFooterPartNameChanged(headerCell, footerCell, headerPartName, footerPartName) {
      [
        { cell: headerCell, partName: headerPartName },
        { cell: footerCell, partName: footerPartName },
      ].forEach(({ cell, partName }) => {
        if (cell) {
          const customParts = cell.__customParts || [];
691       cell.part.remove(...customParts);

          cell.__customParts = partName ? partName.trim().split(' ') : [];
          cell.part.add(...cell.__customParts);
        }
      });
    }

I was able to solve this issue locally by adding a validation of cell.part on this block before invoking the remove and add functions.

    __headerFooterPartNameChanged(headerCell, footerCell, headerPartName, footerPartName) {
      [
        { cell: headerCell, partName: headerPartName },
        { cell: footerCell, partName: footerPartName },
      ].forEach(({ cell, partName }) => {
        if (cell && cell.part) {
          const customParts = cell.__customParts || [];
          cell.part.remove(...customParts);

          cell.__customParts = partName ? partName.trim().split(' ') : [];
          cell.part.add(...cell.__customParts);
        }
      });
    }

However, after solving this line, we had the same thing occur on line 727 of node_modules/@vaadin/grid/src/vaadin-grid-mixin.js.

  ● Test suite failed to run

    TypeError: Cannot read properties of undefined (reading 'add')

      at add (../../node_modules/@vaadin/grid/src/vaadin-grid-mixin.js:727:23)

Which I was able to solve using the same method:

727            if (cell.part) {
728              cell.part.add('cell', `${section}-cell`);
729            }

Because we're unable to reproduce this error in the browser, isolating what is causing this cell.part to be undefined is exceptionally challenging. It would be a huge help if the validation above could be added to just avoid this error form occurring altogether.

Expected outcome

Vaadin grid should render in Jest environment without internal errors. Errors should be the same in Jest as the instance rendering into the browser DOM.

Minimal reproducible example

We haven't been able to reproduce this in any anonymized implementation of vaadin-grid. Because the solution is so trivial, we're hoping that these changes could be added without reproducing in an anonymized implementation.

Steps to reproduce

Again, we're unable to reproduce this with any generic versions of our components.

Environment

Vaadin version(s): @vaadin/grid 24.3.10
OS: MacOS Sonoma 14.4.1

Jest version: 29.7.0

Browsers

No response

@web-padawan
Copy link
Member

The issue is caused by the fact that JSDOM used by Jest is missing support for shadow parts: jsdom/jsdom#3366

We could add a check as you suggest. But in general, we don't specifically support JSDOM environment.
There might be other missing features e.g. ResizeObserver used by vaadin-grid internally.

@rolfsmeds rolfsmeds added enhancement New feature or request test Failing or incorrect test Impact: Low labels May 2, 2024
@j-scola
Copy link
Author

j-scola commented May 2, 2024

Thanks for getting back to me on this. Helpful info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Impact: Low test Failing or incorrect test vaadin-grid
Projects
None yet
Development

No branches or pull requests

3 participants