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

[combobox] Uncaught type errors are thrown #9321

Open
2 of 6 tasks
mpayson opened this issue May 13, 2024 · 3 comments
Open
2 of 6 tasks

[combobox] Uncaught type errors are thrown #9321

mpayson opened this issue May 13, 2024 · 3 comments
Assignees
Labels
3 - installed Issues that have been merged to master branch and are ready for final confirmation. ArcGIS Online Issues logged by ArcGIS Online team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components-react Issues specific to the @esri/calcite-components-react package. estimate - 2 Small fix or update, may require updates to tests. has workaround Issues have a workaround available in the meantime. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@mpayson
Copy link

mpayson commented May 13, 2024

Check existing issues

Actual Behavior

Calcite combobox throws type errors in the console logs

Expected Behavior

No uncaught type errors

Reproduction Sample

https://github.com/mpayson/calcite-debug

Reproduction Steps

  1. Clone the reproduction sample above and checkout the branch combobox-repo
  2. Run the app with developer console open
  3. Click the "Toggle" button
  4. See uncaught error in console

In the reproduction case, the type error is:

TypeError: this.textInput is null
documentClickHandler combobox.tsx:357
hostListenerProxy index.js:2622
ael index.js:2878
addHostEventListeners index.js:2608
addHostEventListeners index.js:2604
registerHost index.js:2808
__registerHost index.js:2358
Combobox2 @esri_calcite-components_dist_components_calcite-combobox.js:3389
React 13
onClick main.jsx:18
React 23
main.jsx:29

In our app, this is the type error, it happens for comboboxes across our app

TypeError: Cannot read properties of null (reading 'value')
at Combobox2.documentClickHandler (combobox.tsx:357:51)
at HTMLDocument. (index.js:2622:39) undefined

Additional notes:

  • Rendering just the combobox as the app root does not reproduce the issue, it seems like it is related to comboboxes being rendered at some point within the app

Reproduction Version

2.8.0

Relevant Info

No response

Regression?

No response

Priority impact

p1 - need for current milestone

Impact

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components

Esri team

ArcGIS Online

@mpayson mpayson added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels May 13, 2024
@github-actions github-actions bot added impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone ArcGIS Online Issues logged by ArcGIS Online team members. calcite-components-react Issues specific to the @esri/calcite-components-react package. labels May 13, 2024
@jcfranco
Copy link
Member

Dug a bit and this is caused by this line. It seems the window click listener is set up and triggered before the component is rendered.

In terms of workaround, you could toggle the component's hidden property vs recreating it:

{(
        <CalciteCombobox hidden={!show}>
          <CalciteComboboxItem value="1" textLabel="One" />
          <CalciteComboboxItem value="2" textLabel="Two" />
        </CalciteCombobox>
      )}

or setting allowCustomValues to true to bypass the error-throwing check and restoring the default after it's created (⚠️quick workaround, needs fine-tuning to follow React best practices⚠️):

function App() {
  const [show, setShow] = useState(false);
  const [workaround, setWorkaround] = useState(true);
  return (
    <>
      <button onClick={() => setShow(!show)}>Toggle</button>
      {show && (
        <CalciteCombobox allowCustomValues={workaround} ref={() =>
          requestAnimationFrame(() => setWorkaround(undefined))}>
          <CalciteComboboxItem value="1" textLabel="One" />
          <CalciteComboboxItem value="2" textLabel="Two" />
        </CalciteCombobox>
      )}
    </>
  );
}

@jcfranco jcfranco added p - medium Issue is non core or affecting less that 60% of people using the library estimate - 2 Small fix or update, may require updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. has workaround Issues have a workaround available in the meantime. and removed needs triage Planning workflow - pending design/dev review. labels May 20, 2024
@jcfranco jcfranco self-assigned this May 20, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. p - high Issue should be addressed in the current milestone, impacts component or core functionality and removed 0 - new New issues that need assignment. p - medium Issue is non core or affecting less that 60% of people using the library labels May 20, 2024
@geospatialem geospatialem added this to the 2024-05-21 April hotfix milestone May 20, 2024
@geospatialem geospatialem removed the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label May 20, 2024
jcfranco added a commit that referenced this issue May 21, 2024
… component is appended to the DOM (#9373)

**Related Issue:** #9321

## Summary

This fixes an issue where references to internal elements were undefined
in the window-level click handler. This would happen because the window
click handler was added when the component was connected, but before its
internals were rendered.
benelan pushed a commit that referenced this issue May 21, 2024
… component is appended to the DOM (#9373)

**Related Issue:** #9321

## Summary

This fixes an issue where references to internal elements were undefined
in the window-level click handler. This would happen because the window
click handler was added when the component was connected, but before its
internals were rendered.
jcfranco added a commit that referenced this issue May 21, 2024
… component is appended to the DOM (#9380)

**Related Issue:** #9321

## Summary

This fixes an issue where references to internal elements were undefined
in the window-level click handler. This would happen because the window
click handler was added when the component was connected, but before its
internals were rendered.

Co-authored-by: JC Franco <jfranco@esri.com>
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels May 23, 2024
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco May 23, 2024
@DitwanP DitwanP added 2 - in development Issues that are actively being worked on. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels May 24, 2024
@DitwanP DitwanP assigned jcfranco and unassigned geospatialem and DitwanP May 24, 2024
jcfranco added a commit that referenced this issue May 28, 2024
…nts) output when a click is emitted when the component is appended to the DOM (#9423)

**Related Issue:** #9321

## Summary

This updates the combobox window-click listener to use an internal state
property for its text instead of getting the value of the internal
input.

The `componentOnReady` util helps normalize waiting for a component to
be ready in both output targets (based on [Ionic's
helper](https://github.com/ionic-team/ionic-framework/blob/5cdfa1aaf47a6160cd1bd2be434dcfa8390b56e1/core/src/utils/helpers.ts#L60-L79)).
This is sufficient for most scenarios, but for this particular case, it
was running earlier than `connectedCallback` due to the emitted click as
the component is appended and initialized, so the component is not yet
rendered by the time the util resolves.

**Note**: there are no accompanying tests as this is not reproducible in
the test environment, which uses the lazy-loaded output.
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels May 28, 2024
@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco May 28, 2024
Copy link
Contributor

Installed and assigned for verification.

benelan pushed a commit that referenced this issue May 29, 2024
…nts) output when a click is emitted when the component is appended to the DOM (#9423)

**Related Issue:** #9321

## Summary

This updates the combobox window-click listener to use an internal state
property for its text instead of getting the value of the internal
input.

The `componentOnReady` util helps normalize waiting for a component to
be ready in both output targets (based on [Ionic's
helper](https://github.com/ionic-team/ionic-framework/blob/5cdfa1aaf47a6160cd1bd2be434dcfa8390b56e1/core/src/utils/helpers.ts#L60-L79)).
This is sufficient for most scenarios, but for this particular case, it
was running earlier than `connectedCallback` due to the emitted click as
the component is appended and initialized, so the component is not yet
rendered by the time the util resolves.

**Note**: there are no accompanying tests as this is not reproducible in
the test environment, which uses the lazy-loaded output.
benelan pushed a commit that referenced this issue May 29, 2024
…nts) output when a click is emitted when the component is appended to the DOM (#9423)

**Related Issue:** #9321

## Summary

This updates the combobox window-click listener to use an internal state
property for its text instead of getting the value of the internal
input.

The `componentOnReady` util helps normalize waiting for a component to
be ready in both output targets (based on [Ionic's
helper](https://github.com/ionic-team/ionic-framework/blob/5cdfa1aaf47a6160cd1bd2be434dcfa8390b56e1/core/src/utils/helpers.ts#L60-L79)).
This is sufficient for most scenarios, but for this particular case, it
was running earlier than `connectedCallback` due to the emitted click as
the component is appended and initialized, so the component is not yet
rendered by the time the util resolves.

**Note**: there are no accompanying tests as this is not reproducible in
the test environment, which uses the lazy-loaded output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - installed Issues that have been merged to master branch and are ready for final confirmation. ArcGIS Online Issues logged by ArcGIS Online team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. calcite-components-react Issues specific to the @esri/calcite-components-react package. estimate - 2 Small fix or update, may require updates to tests. has workaround Issues have a workaround available in the meantime. impact - p1 - need for current milestone User set priority impact status of p1 - need for current milestone p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests

4 participants