From 593ca28b20e7af59e9656e86b564b7889842b7f9 Mon Sep 17 00:00:00 2001 From: Jacob Bandes-Storch Date: Thu, 28 Sep 2023 08:22:05 -0700 Subject: [PATCH] Add support for TypedArrays in User Scripts using @foxglove/schemas (#6890) **User-Facing Changes** Fixed a type incompatibility issue with User Scripts that import from @foxglove/schemas. **Description** Fixes #6821 Fixes FG-4976 Depends on https://github.com/foxglove/schemas/pull/128 Also updated JSON message datatype extraction to treat integer arrays as `Uint32Array` rather than `Float64Array`; this fixed a similar issue as #6821 with an example json-based mcap I had lying around. --- benchmark/package.json | 2 +- packages/mcap-support/package.json | 2 +- .../mcap-support/src/parseJsonSchema.test.ts | 12 +++- packages/mcap-support/src/parseJsonSchema.ts | 25 +++++++- packages/studio-base/package.json | 2 +- .../getMessagePathSearchItems.test.ts | 12 ++-- .../nodeTransformerWorker/transform.test.ts | 57 +++++++++++++++++++ .../typescript/projectConfig.ts | 5 +- yarn.lock | 14 ++--- 9 files changed, 110 insertions(+), 21 deletions(-) diff --git a/benchmark/package.json b/benchmark/package.json index 9bfab2bfcc1..645ea60b8ff 100644 --- a/benchmark/package.json +++ b/benchmark/package.json @@ -8,7 +8,7 @@ "@foxglove/den": "workspace:*", "@foxglove/log": "workspace:*", "@foxglove/rostime": "1.1.2", - "@foxglove/schemas": "1.5.1", + "@foxglove/schemas": "1.6.0", "@foxglove/studio": "workspace:*", "@foxglove/tsconfig": "2.0.0", "@pmmmwh/react-refresh-webpack-plugin": "0.5.11", diff --git a/packages/mcap-support/package.json b/packages/mcap-support/package.json index cee21c195dc..d70c9778812 100644 --- a/packages/mcap-support/package.json +++ b/packages/mcap-support/package.json @@ -33,7 +33,7 @@ "@foxglove/rosmsg": "4.2.2", "@foxglove/rosmsg-serialization": "2.0.2", "@foxglove/rosmsg2-serialization": "2.0.2", - "@foxglove/schemas": "1.5.1", + "@foxglove/schemas": "1.6.0", "@foxglove/wasm-bz2": "0.1.1", "@foxglove/wasm-lz4": "1.0.2", "@foxglove/wasm-zstd": "1.0.1", diff --git a/packages/mcap-support/src/parseJsonSchema.test.ts b/packages/mcap-support/src/parseJsonSchema.test.ts index 7e329f1d279..ca1952f7bc5 100644 --- a/packages/mcap-support/src/parseJsonSchema.test.ts +++ b/packages/mcap-support/src/parseJsonSchema.test.ts @@ -53,6 +53,8 @@ describe("parseJsonSchema", () => { bin: { type: "string", contentEncoding: "base64" }, num: { type: "number" }, int: { type: "integer" }, + uint: { type: "integer", minimum: 0 }, + uint2: { type: "integer", exclusiveMinimum: 0 }, bool: { type: "boolean" }, }, }, @@ -64,7 +66,9 @@ describe("parseJsonSchema", () => { { name: "str", type: "string" }, { name: "bin", type: "uint8", isArray: true }, { name: "num", type: "float64" }, - { name: "int", type: "float64" }, + { name: "int", type: "int32" }, + { name: "uint", type: "uint32" }, + { name: "uint2", type: "uint32" }, { name: "bool", type: "bool" }, ], }, @@ -94,6 +98,8 @@ describe("parseJsonSchema", () => { str: { type: "array", items: { type: "string" } }, num: { type: "array", items: { type: "number" } }, int: { type: "array", items: { type: "integer" } }, + uint: { type: "array", items: { type: "integer", minimum: 0 } }, + uint2: { type: "array", items: { type: "integer", exclusiveMinimum: 0 } }, bool: { type: "array", items: { type: "boolean" } }, }, }, @@ -104,7 +110,9 @@ describe("parseJsonSchema", () => { definitions: [ { name: "str", type: "string", isArray: true }, { name: "num", type: "float64", isArray: true }, - { name: "int", type: "float64", isArray: true }, + { name: "int", type: "int32", isArray: true }, + { name: "uint", type: "uint32", isArray: true }, + { name: "uint2", type: "uint32", isArray: true }, { name: "bool", type: "bool", isArray: true }, ], }, diff --git a/packages/mcap-support/src/parseJsonSchema.ts b/packages/mcap-support/src/parseJsonSchema.ts index e35619a22ce..2e1e78ea368 100644 --- a/packages/mcap-support/src/parseJsonSchema.ts +++ b/packages/mcap-support/src/parseJsonSchema.ts @@ -96,9 +96,19 @@ export function parseJsonSchema( } break; case "number": - case "integer": fields.push({ name: fieldName, type: "float64" }); break; + case "integer": + fields.push({ + name: fieldName, + type: + (typeof fieldSchema.minimum === "number" && fieldSchema.minimum >= 0) || + (typeof fieldSchema.exclusiveMinimum === "number" && + fieldSchema.exclusiveMinimum >= 0) + ? "uint32" + : "int32", + }); + break; case "object": { const nestedTypeName = `${typeName}.${fieldName}`; const postprocessNestedObject = addFieldsRecursive( @@ -134,9 +144,20 @@ export function parseJsonSchema( fields.push({ name: fieldName, type: "string", isArray: true }); break; case "number": - case "integer": fields.push({ name: fieldName, type: "float64", isArray: true }); break; + case "integer": + fields.push({ + name: fieldName, + type: + (typeof itemSchema.minimum === "number" && itemSchema.minimum >= 0) || + (typeof itemSchema.exclusiveMinimum === "number" && + itemSchema.exclusiveMinimum >= 0) + ? "uint32" + : "int32", + isArray: true, + }); + break; case "object": { const nestedTypeName = `${typeName}.${fieldName}`; const postprocessArrayItem = addFieldsRecursive( diff --git a/packages/studio-base/package.json b/packages/studio-base/package.json index 105456ac79b..509dff209f8 100644 --- a/packages/studio-base/package.json +++ b/packages/studio-base/package.json @@ -37,7 +37,7 @@ "@foxglove/rosmsg-serialization": "2.0.2", "@foxglove/rosmsg2-serialization": "2.0.2", "@foxglove/rostime": "1.1.2", - "@foxglove/schemas": "1.5.1", + "@foxglove/schemas": "1.6.0", "@foxglove/studio": "workspace:*", "@foxglove/theme": "workspace:*", "@foxglove/three-text": "0.2.2", diff --git a/packages/studio-base/src/components/TopicList/getMessagePathSearchItems.test.ts b/packages/studio-base/src/components/TopicList/getMessagePathSearchItems.test.ts index 5c1323a9247..7f4240c368d 100644 --- a/packages/studio-base/src/components/TopicList/getMessagePathSearchItems.test.ts +++ b/packages/studio-base/src/components/TopicList/getMessagePathSearchItems.test.ts @@ -295,7 +295,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".circles[:].timestamp.sec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", @@ -308,7 +308,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".circles[:].timestamp.nsec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", @@ -542,7 +542,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".points[:].timestamp.sec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", @@ -555,7 +555,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".points[:].timestamp.nsec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", @@ -854,7 +854,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".texts[:].timestamp.sec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", @@ -867,7 +867,7 @@ describe("getMessagePathSearchItems", () => { "suffix": { "isLeaf": true, "pathSuffix": ".texts[:].timestamp.nsec", - "type": "float64", + "type": "uint32", }, "topic": { "name": "annotations", diff --git a/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/transform.test.ts b/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/transform.test.ts index 697d6daf5c1..decb3a69b9f 100644 --- a/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/transform.test.ts +++ b/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/transform.test.ts @@ -1176,6 +1176,63 @@ describe("pipeline", () => { datatypes: basicDatatypes, outputDatatype: "foxglove.Color", }, + { + description: "Allows TypedArrays when output datatype is from @foxglove/schemas", + sourceCode: ` + import { LaserScan } from "@foxglove/schemas"; + + export const inputs = []; + export const output = "${DEFAULT_STUDIO_NODE_PREFIX}"; + + const publisher = (message: any): LaserScan => { + return { + timestamp: {sec: 0, nsec: 1}, + frame_id: "", + pose: { + position: {x: 0, y: 0, z: 0}, + orientation: {x: 0, y: 0, z: 0, w: 1}, + }, + start_angle: 0, + end_angle: 0, + ranges: new Float64Array([1, 2, 3]), + intensities: new Float32Array([1, 2, 3]), + }; + }; + + export default publisher;`, + datatypes: basicDatatypes, + outputDatatype: "foxglove.LaserScan", + }, + { + description: + "Rejects wrong kind of TypedArray when output datatype is from @foxglove/schemas", + sourceCode: ` + import { LaserScan } from "@foxglove/schemas"; + + export const inputs = []; + export const output = "${DEFAULT_STUDIO_NODE_PREFIX}"; + + const publisher = (message: any): LaserScan => { + return { + timestamp: {sec: 0, nsec: 1}, + frame_id: "", + pose: { + position: {x: 0, y: 0, z: 0}, + orientation: {x: 0, y: 0, z: 0, w: 1}, + }, + start_angle: 0, + end_angle: 0, + ranges: new Float64Array([1, 2, 3]), + intensities: new Uint32Array([1, 2, 3]), + }; + }; + + export default publisher;`, + error: 2322, + errorMessage: expect.stringContaining( + `Type 'Uint32Array' is not assignable to type 'number[] | Float32Array | Float64Array'`, + ), + }, { description: "Should handle deep subtype lookup", sourceCode: ` diff --git a/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/typescript/projectConfig.ts b/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/typescript/projectConfig.ts index 6d74cb747b8..793453933f6 100644 --- a/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/typescript/projectConfig.ts +++ b/packages/studio-base/src/players/UserNodePlayer/nodeTransformerWorker/typescript/projectConfig.ts @@ -28,7 +28,10 @@ import { UserScriptProjectConfig, UserScriptProjectFile } from "./types"; * Generates virtual ts files for each type exported by the @foxglove/schemas package. */ export function generateFoxgloveSchemaDeclarations(): UserScriptProjectFile[] { - const schemas = _.sortBy([...exportTypeScriptSchemas().entries()], ([name]) => name); + const schemas = _.sortBy( + [...exportTypeScriptSchemas({ includeTypedArrays: true }).entries()], + ([name]) => name, + ); const files = schemas.map(([name, sourceCode]) => { return { fileName: `@foxglove/schemas/${name}.ts`, diff --git a/yarn.lock b/yarn.lock index 1864fd24a2d..9135c0710d7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2493,7 +2493,7 @@ __metadata: "@foxglove/rosmsg": 4.2.2 "@foxglove/rosmsg-serialization": 2.0.2 "@foxglove/rosmsg2-serialization": 2.0.2 - "@foxglove/schemas": 1.5.1 + "@foxglove/schemas": 1.6.0 "@foxglove/tsconfig": 2.0.0 "@foxglove/wasm-bz2": 0.1.1 "@foxglove/wasm-lz4": 1.0.2 @@ -2674,13 +2674,13 @@ __metadata: languageName: node linkType: hard -"@foxglove/schemas@npm:1.5.1, @foxglove/schemas@npm:^1.3.1": - version: 1.5.1 - resolution: "@foxglove/schemas@npm:1.5.1" +"@foxglove/schemas@npm:1.6.0, @foxglove/schemas@npm:^1.3.1": + version: 1.6.0 + resolution: "@foxglove/schemas@npm:1.6.0" dependencies: "@foxglove/rosmsg-msgs-common": ^3.0.0 tslib: ^2.5.0 - checksum: 4dd054a318ead308ce980e51eb50c3bcd1c59f9c94707234f8e293bc9b52f2727a21fdad4516f3c4ba0d19ec8058cd60315fe526b90ed9085d52519aa98d12ad + checksum: ff3552a9d22ed47dd05730735abbab4008502e1db76941605b850bddc65b8376a3a613a9885014b2744b4621901b21ee4d0071f899e89248c04165af0226e8a0 languageName: node linkType: hard @@ -2718,7 +2718,7 @@ __metadata: "@foxglove/rosmsg-serialization": 2.0.2 "@foxglove/rosmsg2-serialization": 2.0.2 "@foxglove/rostime": 1.1.2 - "@foxglove/schemas": 1.5.1 + "@foxglove/schemas": 1.6.0 "@foxglove/studio": "workspace:*" "@foxglove/theme": "workspace:*" "@foxglove/three-text": 0.2.2 @@ -8622,7 +8622,7 @@ __metadata: "@foxglove/den": "workspace:*" "@foxglove/log": "workspace:*" "@foxglove/rostime": 1.1.2 - "@foxglove/schemas": 1.5.1 + "@foxglove/schemas": 1.6.0 "@foxglove/studio": "workspace:*" "@foxglove/studio-base": "workspace:*" "@foxglove/tsconfig": 2.0.0