Migrate `urdf` modules to Typescript #907

drewhoener · 2025-08-08T06:59:06Z

Public API Changes
None

Description

Overview

This PR migrates the classes in the urdf module to Typescript. This PR is slightly more involved as there's a lot of interdependency between the URDF elements, but other than that it's still largely trivial.

Add TS enum for Urdf Type
Add an enum to reduce magic strings when accessing attributes
Extract logic for getting element origin into a separate function in UrdfUtils.ts
Add stricter type checking for getting attributes from XML
Use for-of loops added in ES6, building off my base PR. It's hit the "widely supported" threshold of 95% so there's really no reason not to use it.

Going to leave converting tests for another PR, but they all pass in my docker container

Depends on #906

GitHub doesn't have "dependent PRs" so all previous commits are included here. Might not be worth reviewing until dependencies are merged

EzraBrooks

GitHub is being GitHub here and the commit where you used git mv to ensure history continuity is not being represented in the PR diff. We might need to break that into its own PR (knowing it will probably break the build).

drewhoener · 2025-08-08T18:54:50Z

GitHub is being GitHub here and the commit where you used git mv to ensure history continuity is not being represented in the PR diff. We might need to break that into its own PR (knowing it will probably break the build).

GitHub's diff viewer might have a different threshold for what it considers a different file? At least locally with git in my terminal I can see the file history which is what I was hoping to preserve

EzraBrooks · 2025-08-08T18:55:22Z

Yeah, unfortunately it looks like if I squash-merge this it will lose the history :(

drewhoener · 2025-08-08T18:57:33Z

Yeah, unfortunately it looks like if I squash-merge this it will lose the history :(

Created at #910

…n generation for PR Signed-off-by: Drew Hoener <[email protected]>

…references. Signed-off-by: Drew Hoener <[email protected]>

Add enum of Urdf Attributes to remove magic strings when getting attributes from xml Signed-off-by: Drew Hoener <[email protected]>

Signed-off-by: Drew Hoener <[email protected]>

…ng warning. Signed-off-by: Drew Hoener <[email protected]>

…xtract origin parsing logic to UrdfUtils. Signed-off-by: Drew Hoener <[email protected]>

Signed-off-by: Drew Hoener <[email protected]>

…d isElement check in UrdfUtils Signed-off-by: Drew Hoener <[email protected]>

…since ES6 provides it Signed-off-by: Drew Hoener <[email protected]>

…ith for-of, and improve type safety Signed-off-by: Drew Hoener <[email protected]>

EzraBrooks

LGTM. Thanks for putting the effort in to doing this incrementally so I can scan it more easily!

EzraBrooks · 2025-08-08T19:10:36Z

src/urdf/UrdfColor.ts

-      this.a = parseFloat(rgba[3]);
+    const rgba: Optional<string[]> = xml.getAttribute(UrdfAttrs.Rgba)?.split(' ');
+    if (!rgba || rgba.length !== 4) {
+      return;


Refactoring JS into TS always uncovers extremely odd control flow like this.. early return from a constructor, lol 🤦🏻

EzraBrooks · 2025-08-08T19:15:38Z

src/urdf/UrdfTypes.ts

+export enum UrdfAttrs {
+  Name = 'name',
+  Type = 'type',
+  Parent = 'parent',
+  Link = 'link',
+  Child = 'child',
+  Limit = 'limit',
+  Upper = 'upper',
+  Lower = 'lower',
+  Origin = 'origin',
+  Xyz = 'xyz',
+  Rpy = 'rpy',
+  Size = 'size',
+  Rgba = 'rgba',
+  Length = 'length',
+  Radius = 'radius',
+  Visuals = 'visual',
+  Texture = 'texture',
+  Filename = 'filename',
+  Color = 'color',
+  Geometry = 'geometry',
+  Material = 'material',
+  Scale = 'scale',
+}


This smells odd to me - I don't feel like we should need this enum to keep track of the magic strings (because each string would only be used once), but I understand you're just refactoring the code in-place here.

The URDF stuff could probably use a refactor soon. I've been using https://github.com/gkjohnson/urdf-loaders for years because I found the URDF implementation here roslibjs and in ros3djs too janky.

Not in the scope of this PR anyhow!

(also honestly maybe roslibjs and ros3djs should be merged into one package now that most of the world uses bundlers in their javascript apps and can tree-shake out unnecessary dependencies)

drewhoener · 2025-08-08T19:19:52Z

LGTM. Thanks for putting the effort in to doing this incrementally so I can scan it more easily!

Yeah of course! Glad I finally have the time to get it done, I've been wanting to for a while. I feel like it's needed a bit of modernization.
I probably won't have the next one until next week sometime, I need to think out the cbor/event emitter shenanigans and how that's all going to work.

Thanks for being so responsive with my wave of PRs lol

EzraBrooks · 2025-08-08T19:51:20Z

Yeah, I kept meaning to come back and do this refactor myself after I added Vite and tsc but never had time and at least in my application the JSDoc-generated types have been "fine" in the meantime

drewhoener force-pushed the feature/typescript-urdf-module branch from 1567f32 to e5824be Compare August 8, 2025 06:59

drewhoener mentioned this pull request Aug 8, 2025

Convert library to Typescript Roadmap #904

Open

13 tasks

drewhoener force-pushed the feature/typescript-urdf-module branch 4 times, most recently from 19b3e78 to 8d84634 Compare August 8, 2025 18:44

EzraBrooks reviewed Aug 8, 2025

View reviewed changes

drewhoener force-pushed the feature/typescript-urdf-module branch from 8d84634 to 7e1189c Compare August 8, 2025 18:52

drewhoener added 12 commits August 8, 2025 15:08

revert: chore(vite): Temporarily disable type checking and declaratio…

cd1a8d1

…n generation for PR Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfTypes): Convert UrdfTypes to TypeScript enum and update …

68117be

…references. Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfBox, UrdfTypes): Convert UrdfBox to TypeScript.

fe674c6

Add enum of Urdf Attributes to remove magic strings when getting attributes from xml Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfColor): Convert UrdfColor to TypeScript.

e115cfb

Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfCylinder): Convert UrdfCylinder to TypeScript, fix parsi…

55b135a

…ng warning. Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfJoint, UrdfUtils): Convert UrdfJoint to TypeScript and e…

202ed57

…xtract origin parsing logic to UrdfUtils. Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfMaterial): Convert UrdfMaterial to TypeScript.

388eced

Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfMesh): Convert UrdfMesh to TypeScript.

4ac34b4

Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfSphere): Convert UrdfSphere to TypeScript.

9ab99a2

Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfVisual, UrdfUtils): Convert UrdfVisual to TypeScript, ad…

b8f38f0

…d isElement check in UrdfUtils Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfLink): Convert UrdfLink to TypeScript, loop with for-of …

539061d

…since ES6 provides it Signed-off-by: Drew Hoener <[email protected]>

refactor(UrdfModel): Convert UrdfModel to TypeScript, replace loops w…

71a85c5

…ith for-of, and improve type safety Signed-off-by: Drew Hoener <[email protected]>

drewhoener force-pushed the feature/typescript-urdf-module branch from 7e1189c to 71a85c5 Compare August 8, 2025 19:09

EzraBrooks approved these changes Aug 8, 2025

View reviewed changes

EzraBrooks merged commit 18fbe4c into RobotWebTools:develop Aug 8, 2025
2 checks passed

drewhoener deleted the feature/typescript-urdf-module branch August 8, 2025 19:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate `urdf` modules to Typescript #907

Migrate `urdf` modules to Typescript #907

Uh oh!

drewhoener commented Aug 8, 2025 •

edited

Loading

Uh oh!

EzraBrooks left a comment

Uh oh!

drewhoener commented Aug 8, 2025

Uh oh!

EzraBrooks commented Aug 8, 2025

Uh oh!

drewhoener commented Aug 8, 2025

Uh oh!

EzraBrooks left a comment

Uh oh!

EzraBrooks Aug 8, 2025

Uh oh!

EzraBrooks Aug 8, 2025

Uh oh!

EzraBrooks Aug 8, 2025

Uh oh!

Uh oh!

drewhoener commented Aug 8, 2025 •

edited

Loading

Uh oh!

EzraBrooks commented Aug 8, 2025

Uh oh!

Uh oh!

Migrate urdf modules to Typescript #907

Migrate urdf modules to Typescript #907

Uh oh!

Conversation

drewhoener commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Contents

Uh oh!

EzraBrooks left a comment

Choose a reason for hiding this comment

Uh oh!

drewhoener commented Aug 8, 2025

Uh oh!

EzraBrooks commented Aug 8, 2025

Uh oh!

drewhoener commented Aug 8, 2025

Uh oh!

EzraBrooks left a comment

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

EzraBrooks Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

drewhoener commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EzraBrooks commented Aug 8, 2025

Uh oh!

Uh oh!

Migrate `urdf` modules to Typescript #907

Migrate `urdf` modules to Typescript #907

drewhoener commented Aug 8, 2025 •

edited

Loading

drewhoener commented Aug 8, 2025 •

edited

Loading