search

Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
282 stars 77 forks source link

`TypeError` in `dropdown-group` when disconnected early #10028

Open nwhittaker opened 1 month ago

nwhittaker commented 1 month ago

Check existing issues

Actual Behavior

If a <calcite-dropdown-group> is removed from the DOM before it's done loading, it can throw a TypeError: Cannot read properties of null (reading 'querySelectorAll').

Expected Behavior

It does not throw an error.

Reproduction Sample

https://codepen.io/nwhittaker-esri/pen/zYVdgqQ

Reproduction Steps

  1. Visit the repro and open the devtools console
  2. Click the Toggle dropdown group button and observe the TypeError log appears in the console

Reproduction Version

2.11.1

Relevant Info

Stack trace:

TypeError: Cannot read properties of null (reading 'querySelectorAll')
    at S.getGroupPosition (dropdown-group.tsx:166:29)
    at S.componentWillLoad (dropdown-group.tsx:85:31)
    at _e (index.js:1853:23)
    at ze (index.js:1649:22)
    at Array.r (index.js:1626:26)
    at M (index.js:185:13)
    at P (index.js:227:5)

I suspect treating this.el.parentElement here as optional would be sufficient: https://github.com/Esri/calcite-design-system/blob/59c91c9363fa1041e7ffec8a9384ef464fe05233/packages/calcite-components/src/components/dropdown-group/dropdown-group.tsx#L166

Regression?

No response

Priority impact

impact - p2 - want for an upcoming milestone

Impact

The impact is largely with adding complexity to writing automated tests. In cases where the component is rendered as part of an acceptance test, its presence may not be important. However, the test still needs to wait for it to hydrate before performing an action that has the side-effect of removing the component.

Calcite package

Esri team

ArcGIS Field Apps

driskull commented 1 month ago

I suspect treating this.el.parentElement here as optional would be sufficient:

Yeah I think that is likely the issue:

private getGroupPosition(): number {
    return Array.prototype.indexOf.call(
      this.el.parentElement.querySelectorAll("calcite-dropdown-group"),
      this.el,
    );
  }

Its looking for a parent element when its already been disconnected.

This should probably be refactored to not look up the DOM to get the group position. The group position wouldn't update if the group was moved position either.