fix compile error

[Yancey2023]

Public API Changes

The xml parameter in UrdfModelOptions changed from type Element | undefined to Element | null

Description

fix a compile error in new version of xmldom:

src/urdf/UrdfModel.js:35:7 - error TS2322: Type 'Element | null' is not assignable to type 'Element | null | undefined'.
  Type 'Element' is missing the following properties from type 'Element': classList, className, clientHeight, clientLeft, and 103 more.

35       xmlDoc = parser.parseFromString(string, MIME_TYPE.XML_TEXT).documentElement;

[Yancey2023]

To reproduce this compile error, clone this repo and run npm install.

[MatthijsBurgh]

Your change breaks the CI

[Yancey2023]

Sorry about that. I only tested locally. I will try to fix it.

[Yancey2023]

@MatthijsBurgh fixed

[MatthijsBurgh]

@Yancey2023 Also make sure you update the lock file

[Yancey2023]

Thanks for your review. I have update the lock file.

[EzraBrooks]

Ah, did we never add the typescript compiler to CI here? I thought we had. We should probably add that to prevent dependabot version bumps from breaking the build.

[EzraBrooks]

Actually, I see that the Vite build is running in CI here.

roslibjs/test/build.bash

Line 28 in 2af39fa

npm run build

In what situation are you encountering this build issue? I am not seeing an issue in CI or on my local machine.

[Yancey2023]

The issue arises because the dependency versions in package.json are prefixed with a caret (^), which means any version above this is acceptable. As a result, when my computer has newer versions of dependencies, it uses those updated dependencies. However, these newer versions may be incompatible with 'roslib', which can lead to compilation errors.

[Yancey2023]

Initially, I wasn't know the reason. Therefore, the PR initially only fixed the compilation errors without updating the dependency versions. Since the CI's npm uses caching, it didn’t use the newest dependency versions, causing the CI to initially fail.

[Yancey2023]

I think others have also encountered this issue: #838 (comment)

[Yancey2023]

There is also a PR that attempts to fix this issue, but it does so in an wrong way: #870

[MatthijsBurgh]

I am not sure whether we need all dependency bumps. Only bump dependencies of which the older version is not working anymore

[Yancey2023]

I'm not sure it is a good idea because npm will try to install the newest version as default when version is prefixed with a caret (^) and the depencedy is not cached.

[Yancey2023]

Hello. I can't understand why we're not using the newest version. Can you show me more details?

[MatthijsBurgh]

Dependencies tell what you need, the lock file tells what you use.
So if we don't need a newer version, don't bump it. Improves compatibility for dependents

[Yancey2023]

Sorry for the late reply and my misunderstanding. Everything is done now and I hope you can feel free to review it.

[MatthijsBurgh]

@Yancey2023 Please resolve the conflicts

[Yancey2023]

The conflicts is resolved so you can review now. By the way, I just notice that roslibjs was migrated to typescript and I think it's wonderful. I really love it.