#1320 Add module TypeScript definitions #2282

samueldmeyer · 2024-12-20T23:07:22Z

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

srikant-ch5 · 2025-01-08T05:52:28Z

Hi @samueldmeyer Thanks for the PR.

Could you please fix the DCO issue and also attach the issue 1320 like below in the PR above Dev Certificate ?

samueldmeyer · 2025-01-08T17:38:26Z

@srikant-ch5 I've fixed the DCO issue, updated the message, and rebased on the commits added since I started this PR

samueldmeyer · 2025-01-22T23:50:50Z

@srikant-ch5 Is there anything else I need to do for this pull request?

tomlyn · 2025-01-23T04:54:01Z

Hi @samueldmeyer -- I think I was supposed to look through this. Apologies for not doing so, I've been concentrating on some of the features you asked for.

At first glance I wonder whether we need some tests (presumably Jest tests) to test at least some of this?

Also, I think @matthoward366, @nmgokhale and @caritaou should review the Common Properties parts of this since they will need to maintain this Type script API going forward.

Signed-off-by: Samuel Meyer <[email protected]>

samueldmeyer · 2025-01-23T22:30:59Z

@tomlyn I added tests using tsd, which is the standard for TypeScript definitions inside the library. I didn't test everything because a lot of that would simply be a repeat of what is in the definitions, but I did test the parts that are more complicated and generally made it extensible for adding tests. I added the tests to npm test, but you could tell me another way you want it run.

Signed-off-by: Samuel Meyer <[email protected]>

Signed-off-by: CTomlyn <[email protected]>

…ies.ts Signed-off-by: CTomlyn <[email protected]>

Signed-off-by: Samuel Meyer <[email protected]>

srikant-ch5 · 2025-02-06T15:02:55Z

@samueldmeyer I have updated test-harness to use Typescript Definitions by updating below in test harness package.json to use index.d.ts from common-canvas

After making few more changes to the setup I can see that Common Canvas and Common Properties are working fine.

Screen.Recording.2025-02-06.at.8.30.56.PM.mov

If there are any other ways to test this PR, please feel free to let me know so that I can test the functionality.

samueldmeyer · 2025-02-06T16:32:22Z

@srikant-ch5 The only way to test TypeScript definitions is to use it with TypeScript code. For that purpose, we have a test file: index.test-d.ts. You can tell me if there are any additional tests we should add to verify methods have correct types in canvas_modules/common-canvas/types/common-properties-controller.ts

tomlyn · 2025-02-07T18:00:27Z

HI @srikant-ch5 -- I am making some changes to this PR. I recommend you pause your work on it for a while, at least until I have pushed my changes. If you have time you could start to think about what tests we could add to index.test-d.ts as suggested by @samueldmeyer

… TS files Signed-off-by: CTomlyn <[email protected]>

samueldmeyer · 2025-02-14T17:06:24Z

canvas_modules/common-canvas/types/common-canvas.ts

+// from JSON. So this creates a PipelineFlowPalette that allows validation.
+export type RelaxedPipelineFlowPalette = Omit<PipelineFlowPalette, 'version' | 'categories'> & {
+  version: string;
+  categories: unknown[];


Both version and categories need to be optional to allow an existing PipelineFlowPalette to count as valid

samueldmeyer · 2025-02-14T17:09:32Z

canvas_modules/common-canvas/types/common-canvas-schema-types/canvas-info-v3.ts

+  /**
+   * Unique identifier for canvas node within the current pipeline
+   */
+  id?: string;


Is this really optional? I thought it was required, but please tell me if it is not.

samueldmeyer · 2025-02-14T17:22:02Z

canvas_modules/common-canvas/types/common-canvas.ts

+  menu: ContextMenuEntry[];
+  toolbarItem: boolean;


Both of these should be optional.
Also, are submenu and menu undocumented? https://elyra-ai.github.io/canvas/03.03.01-context-menu-handler/#customizing-the-default-context-menu

samueldmeyer · 2025-02-14T17:31:30Z

canvas_modules/common-canvas/types/common-canvas.ts

   */
  deleteLink(
-    link: string | CanvasLink | Record<string, unknown>,
+    link: CanvasLink,


In canvas_modules/common-canvas/__tests__/object-model/canvas-controller-api-test.js this is used with only a string, and that worked when I tried it. Is that a deprecated signature?

samueldmeyer · 2025-02-14T17:36:23Z

canvas_modules/common-canvas/types/common-canvas-schema-types/canvas-info-v3.ts

+  x_pos?: number;
+  /**
+   * y-position of this canvas execution node.
+   */
+  y_pos?: number;


Are x_pos and y_pos optional? They appeared to be required, but you can tell me if they are not.

samueldmeyer · 2025-02-14T20:24:50Z

canvas_modules/common-canvas/locales/common-properties/locales/index.d.ts

+ * limitations under the License.
+ */
+
+declare const messages: Record<string, string>;


On testing, I realize this should be Record<string, Record<string, string>>;

samueldmeyer · 2025-02-14T20:24:56Z

canvas_modules/common-canvas/locales/notification-panel/locales/index.d.ts

+ * limitations under the License.
+ */
+
+declare const messages: Record<string, string>;


On testing, I realize this should be Record<string, Record<string, string>>;

samueldmeyer · 2025-02-14T20:25:00Z

canvas_modules/common-canvas/locales/toolbar/locales/index.d.ts

+ * limitations under the License.
+ */
+
+declare const messages: Record<string, string>;


On testing, I realize this should be Record<string, Record<string, string>>;

samueldmeyer · 2025-02-14T20:25:19Z

canvas_modules/common-canvas/locales/palette/locales/index.d.ts

+ * limitations under the License.
+ */
+
+declare const messages: Record<string, string>;


On testing, I realize this should be Record<string, Record<string, string>>;

samueldmeyer · 2025-02-14T20:53:49Z

canvas_modules/common-canvas/types/common-canvas.ts

+  selectedObjects: Record<string, unknown>[];
+  /** @deprecated */
+  selectedObjectIds: string[];
+  [key: string]: unknown;


Based on SVGCanvasRenderer.openContextMenu, it seems like type, targetObject, pipelineId and a few others are included if the editSource is contextmenu. Is that a level of detail we should include? Or should we skip that for now? I only bring this up because you used targetObject in editActionHandler in canvas_modules/harness/src/client/components/custom-canvases/prompt/prompt-canvas.jsx

samueldmeyer force-pushed the add-type-defs branch from b5d26eb to 3916290 Compare December 20, 2024 23:09

samueldmeyer changed the title ~~#1320 Add module TypeScript definitions~~ #1320 [WIP] Add module TypeScript definitions Dec 20, 2024

samueldmeyer force-pushed the add-type-defs branch from 3916290 to 6eee5aa Compare December 20, 2024 23:31

samueldmeyer changed the title ~~#1320 [WIP] Add module TypeScript definitions~~ #1320 Add module TypeScript definitions Dec 20, 2024

samueldmeyer force-pushed the add-type-defs branch 2 times, most recently from 382ada2 to 65fbffe Compare January 8, 2025 17:01

elyra-ai#1320 Add module TypeScript definitions

a848797

Signed-off-by: Samuel Meyer <[email protected]>

samueldmeyer force-pushed the add-type-defs branch from 9d8c847 to b79c89a Compare January 23, 2025 22:26

add tests for TypeScript definitions

df90abe

Signed-off-by: Samuel Meyer <[email protected]>

samueldmeyer force-pushed the add-type-defs branch from b79c89a to df90abe Compare January 23, 2025 22:28

add new config enableSingleClickAddFromPalette

c687c20

Signed-off-by: Samuel Meyer <[email protected]>

samueldmeyer force-pushed the add-type-defs branch from 7702209 to c687c20 Compare January 25, 2025 00:27

tomlyn and others added 3 commits January 28, 2025 15:18

Copyright dates

ba92f9c

Signed-off-by: CTomlyn <[email protected]>

Rename and refactor files and create common-canvas.ts and commpropert…

8597038

…ies.ts Signed-off-by: CTomlyn <[email protected]>

update toolbar action item interface

afefd24

Signed-off-by: Samuel Meyer <[email protected]>

samueldmeyer force-pushed the add-type-defs branch from 2de314b to afefd24 Compare February 5, 2025 18:21

Remove TS generation, improve common-canvas TS based on new generated…

8760624

… TS files Signed-off-by: CTomlyn <[email protected]>

samueldmeyer commented Feb 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1320 Add module TypeScript definitions #2282

#1320 Add module TypeScript definitions #2282

samueldmeyer commented Dec 20, 2024 •

edited

Loading

srikant-ch5 commented Jan 8, 2025

samueldmeyer commented Jan 8, 2025

samueldmeyer commented Jan 22, 2025

tomlyn commented Jan 23, 2025

samueldmeyer commented Jan 23, 2025

srikant-ch5 commented Feb 6, 2025

samueldmeyer commented Feb 6, 2025

tomlyn commented Feb 7, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

samueldmeyer Feb 14, 2025

#1320 Add module TypeScript definitions #2282

Are you sure you want to change the base?

#1320 Add module TypeScript definitions #2282

Conversation

samueldmeyer commented Dec 20, 2024 • edited Loading

srikant-ch5 commented Jan 8, 2025

samueldmeyer commented Jan 8, 2025

samueldmeyer commented Jan 22, 2025

tomlyn commented Jan 23, 2025

samueldmeyer commented Jan 23, 2025

srikant-ch5 commented Feb 6, 2025

samueldmeyer commented Feb 6, 2025

tomlyn commented Feb 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samueldmeyer commented Dec 20, 2024 •

edited

Loading