adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.12k stars 1.06k forks source link

Combobox crashes on ios 18 (childNodes is not supported) #6600

Closed aulonm closed 5 days ago

aulonm commented 1 week ago

Provide a general summary of the issue here

While looking at some sentry issues on our site, I saw that one of our site was crashing hard on an ios 18 device. This made me check with your docs as well and saw that the same thing happens there.

I know that iOS 18 is still in developer beta, so this could maybe be fixed automatically by Apple, not entirely sure

Stack trace:

Error: childNodes is not supported
  at get $7135fc7d473fd974$export$f5d856d854e74713.childNodes(../../node_modules/react-aria-components/dist/Collection.mjs:22:1)
  at visit(../../node_modules/@react-stately/list/dist/ListCollection.mjs:53:1)
  at new $a02d57049d202695$export$d085fb9e920b5ca7(../../node_modules/@react-stately/list/dist/ListCollection.mjs:55:1)
  at filteredCollection(../../node_modules/@react-stately/combobox/dist/useComboBoxState.mjs:269:20)
  at Rg(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:124:348)
  at state(../../node_modules/@react-stately/combobox/dist/useComboBoxState.mjs:49:48)
  at state(../../node_modules/@react-stately/combobox/dist/useComboBoxState.mjs:267:1)
  at Tf(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:108:419)
  at Kh(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:160:170)
  at al(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:278:18)
  at Xk(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:264:322)
  at <anonymous>(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:262:29)
  at ke(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:262:29)
  at le(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:255:92)
  at ie(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:77:247)
  at <anonymous>(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:270:51)
  at qe(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:270:58)
  at Kk(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:255:428)
  at xe(../../node_modules/next/dist/compiled/react-dom/cjs/react-dom.production.min.js:254:248)
  at P(../../node_modules/next/dist/compiled/scheduler/cjs/scheduler.production.min.js:14:246)

Image of the sentry trace: image

🤔 Expected Behavior?

Should work the same way as ios 17

😯 Current Behavior

It crashes and renders "an error occured while rendering the example" in the docs

image

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

Need to have an iOS 18 Developer Beta device.

Version

react-aria-components@1.2.1

What browsers are you seeing the problem on?

Chrome, Safari

If other, please specify.

No response

What operating system are you using?

iOS 18 Developer Beta

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

reidbarber commented 6 days ago

Thanks for reporting this. I think we may just need to replace uses of node.childNodes with this.getChildren(node.key) here and elsewhere:

https://github.com/adobe/react-spectrum/blob/4e3af379e569faac3b374e0ed5a98b2a19cd92c3/packages/%40react-stately/list/src/ListCollection.ts#L27-L30

I'm downloading the Simulator beta to try to test this.

reidbarber commented 6 days ago

I couldn't reproduce this on a simulated iOS 18 device, so maybe we can take a look closer to GA, or get someone with a real device to try the fix.

aulonm commented 5 days ago

Sorry for a late reply. It actually looks like it was a minor bug with iOS 18 Beta 1. It works in iOS beta 2 🤷‍♂️

You can close this as fixed by Apple? I'll open a new issue if they end up changing it again before GA