Upgrade Matter.js to 0.16#2429
Conversation
Deploying gladys-plus with
|
| Latest commit: |
97df3e6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://585e119e.gladys-plus.pages.dev |
| Branch Preview URL: | https://updgrade-matter-0-16-8.gladys-plus.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2429 +/- ##
==========================================
- Coverage 98.81% 98.81% -0.01%
==========================================
Files 1007 1007
Lines 17534 17674 +140
==========================================
+ Hits 17326 17464 +138
- Misses 208 210 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
#4040 Bundle Size — 11.41MiB (0%).97df3e6(current) vs 25cbcdd master#4039(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
| Current #4040 |
Baseline #4039 |
|
|---|---|---|
 Initial JS |
6.4MiB |
6.4MiB |
| Initial CSS |
310.49KiB |
310.49KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
51 |
51 |
| Assets |
179 |
179 |
| Modules |
1643 |
1643 |
| Duplicate Modules |
21 |
21 |
| Duplicate Code |
0.94% |
0.94% |
| Packages |
136 |
136 |
| Duplicate Packages |
2 |
2 |
Bundle size by type no changes
| Current #4040 |
Baseline #4039 |
|
|---|---|---|
| JS |
8.3MiB |
8.3MiB |
| IMG |
2.66MiB |
2.66MiB |
| CSS |
328.34KiB |
328.34KiB |
| Fonts |
93.55KiB |
93.55KiB |
| Other |
18.82KiB |
18.82KiB |
| HTML |
13.58KiB |
13.58KiB |
Bundle analysis report Branch updgrade-matter-0-16-8 Project dashboard
Generated by RelativeCI Documentation Report issue
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR bumps Matter package versions and replaces direct device property access ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/services/matter/lib/matter.setValue.js (1)
29-35:⚠️ Potential issue | 🔴 CriticalChild endpoint traversal incompatible with Matter.js 0.16.8 API.
The
childEndpointsproperty does not exist in the new Matter.js 0.16.8 endpoint API. In the current architecture, child endpoints should be accessed viaendpoint.parts(usingparts.get(id)for lookup orhasPartsto check existence), not through achildEndpointsproperty orgetChildEndpoints()method.The recursion at lines 29–35 requires migration to the new Parts API:
- Replace
parentDevice.childEndpointswithparentDevice.parts- Use
parts.get(deviceNumber)or iterate viapartsdirectly- Consider whether
endpoint.visit()can replace the recursive traversal entirelyThis is more than a compatibility layer—it requires refactoring the traversal logic to align with the new endpoint composition model.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/matter/lib/matter.setValue.js` around lines 29 - 35, The traversal uses the removed parentDevice.childEndpoints API; update findDeviceRecursively to use the new Parts API: check parentDevice.parts (or parentDevice.hasParts()) instead of childEndpoints, locate a child via parentDevice.parts.get(deviceNumber) or iterate parentDevice.parts to find matching part, and recurse with that part and remainingPath; alternatively refactor to use endpoint.visit() to traverse parts if that simplifies the recursion. Ensure all references to childEndpoints/getChildEndpoints() are replaced and adjust any existence checks to use hasParts()/parts accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/package.json`:
- Line 49: Update the dependency entry for `@matter/main` in package.json to
reference the newer 0.16.9 release (e.g., change the version string from
"^0.16.8" to "^0.16.9" or ensure the caret range allows the latest 0.16.9) so
the project pulls the latest patch; locate the "@matter/main" line in
package.json and modify the version accordingly, then run your package manager
install to verify compatibility.
---
Outside diff comments:
In `@server/services/matter/lib/matter.setValue.js`:
- Around line 29-35: The traversal uses the removed parentDevice.childEndpoints
API; update findDeviceRecursively to use the new Parts API: check
parentDevice.parts (or parentDevice.hasParts()) instead of childEndpoints,
locate a child via parentDevice.parts.get(deviceNumber) or iterate
parentDevice.parts to find matching part, and recurse with that part and
remainingPath; alternatively refactor to use endpoint.visit() to traverse parts
if that simplifies the recursion. Ensure all references to
childEndpoints/getChildEndpoints() are replaced and adjust any existence checks
to use hasParts()/parts accordingly.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/services/matter/lib/matter.handleNode.js (1)
134-137:⚠️ Potential issue | 🟠 MajorGuard
deviceData.basicInformationbefore callinghandleDevice.We only bail out when
nodeDetail.deviceDatais missing, but Line 146 still passesnodeDetail.deviceData.basicInformationandhandleDevicedereferences it immediately on Line 20. A partially populated node payload will still throw before the new bridged-attribute fallbacks even run.Proposed fix
async function handleNode(nodeDetail) { logger.debug(`Matter: Handling node ${nodeDetail.nodeId}`); - if (!nodeDetail.deviceData) { - logger.warn(`Matter: Node ${nodeDetail.nodeId} has no device data`); + if (!nodeDetail.deviceData?.basicInformation) { + logger.warn(`Matter: Node ${nodeDetail.nodeId} has no device basic information`); return; } + const basicInformation = nodeDetail.deviceData.basicInformation; const node = await this.commissioningController.getNode(nodeDetail.nodeId); this.nodesMap.set(nodeDetail.nodeId, node); const devices = node.getDevices(); const boundListenToStateChange = this.listenToStateChange.bind(this); await Promise.each(devices, async (device) => { @@ await handleDevice( nodeDetail.nodeId, - nodeDetail.deviceData.basicInformation, + basicInformation, node, device, this.devices,Also applies to: 144-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/matter/lib/matter.handleNode.js` around lines 134 - 137, The code currently only checks nodeDetail.deviceData and then calls handleDevice with nodeDetail.deviceData.basicInformation, which can be undefined and cause a crash; update the guard to ensure deviceData.basicInformation exists (or provide a safe default) before calling handleDevice (references: nodeDetail, deviceData.basicInformation, handleDevice) — either return/log when basicInformation is missing or pass a validated/default object so handleDevice and its immediate dereferences are safe.
🧹 Nitpick comments (1)
server/services/matter/lib/matter.handleNode.js (1)
40-40: Include the failing endpoint in these warnings.
nodeIdis shared by every bridged child on the node, so these messages still won't tell you which endpoint failed when several children are involved. Addingdevice.numberhere, or computingnewDevicePathbefore the reads, would make the warnings actionable.Also applies to: 49-49, 58-58, 67-67, 76-76, 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/services/matter/lib/matter.handleNode.js` at line 40, The warning messages using logger.warn currently only include nodeId so they are ambiguous for bridged children; update each warn at the spots referencing nodeId (the calls around reading bridged vendorName, productName, deviceType, deviceName, and vendorId) to also include the failing endpoint by computing newDevicePath (or reading device.number) before the reads and interpolating either newDevicePath or device.number into the log message (i.e., change the logger.warn calls that mention nodeId and e.message to include device.number or newDevicePath alongside nodeId so the warnings uniquely identify which bridged child failed).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@server/services/matter/lib/matter.handleNode.js`:
- Around line 134-137: The code currently only checks nodeDetail.deviceData and
then calls handleDevice with nodeDetail.deviceData.basicInformation, which can
be undefined and cause a crash; update the guard to ensure
deviceData.basicInformation exists (or provide a safe default) before calling
handleDevice (references: nodeDetail, deviceData.basicInformation, handleDevice)
— either return/log when basicInformation is missing or pass a validated/default
object so handleDevice and its immediate dereferences are safe.
---
Nitpick comments:
In `@server/services/matter/lib/matter.handleNode.js`:
- Line 40: The warning messages using logger.warn currently only include nodeId
so they are ambiguous for bridged children; update each warn at the spots
referencing nodeId (the calls around reading bridged vendorName, productName,
deviceType, deviceName, and vendorId) to also include the failing endpoint by
computing newDevicePath (or reading device.number) before the reads and
interpolating either newDevicePath or device.number into the log message (i.e.,
change the logger.warn calls that mention nodeId and e.message to include
device.number or newDevicePath alongside nodeId so the warnings uniquely
identify which bridged child failed).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ac2f814-3ff5-455b-8548-fc7adecc159d
📒 Files selected for processing (1)
server/services/matter/lib/matter.handleNode.js
|
Close in favor of #2501 |
1 participant
Pull Request check-list
To ensure your Pull Request can be accepted as fast as possible, make sure to review and check all of these items:
npm teston both front/server)npm run eslinton both front/server)npm run prettieron both front/server)npm run compare-translationson front)front/src/config/demo.js) so that the demo website is working without a backend? (if needed) See https://demo.gladysassistant.com.NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.
Description of change
Please provide a description of the change here. It's always best with screenshots, so don't hesitate to add some!
Summary by CodeRabbit
Chores
Refactor
Bug Fixes / Improvements
Tests