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

j-scola commented 2 months ago

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 commented 2 months ago

The issue is caused by the fact that JSDOM used by Jest is missing support for shadow parts: https://github.com/jsdom/jsdom/issues/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.

j-scola commented 2 months ago

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

vaadin / web-components