Support field type #3

ryanbas21 · 2025-01-09T22:14:00Z

JIRA Ticket

https://pingidentity.atlassian.net/browse/SDKS-3655

Description

Adds support for a few fields from davinci connectors like dropdown, radio, flowlink, etc.

nx-cloud · 2025-01-09T22:14:48Z

View your CI Pipeline Execution ↗ for commit ee4cb61.

Command	Status	Duration	Result
`nx affected -t typecheck build lint test docs e...`	✅ Succeeded	<1s	View ↗
`nx-cloud record -- nx format:check`	✅ Succeeded	9s	View ↗

☁️ Nx Cloud last updated this comment at 2025-01-30 20:38:47 UTC

codecov-commenter · 2025-01-09T22:31:43Z

Codecov Report

Attention: Patch coverage is 84.21053% with 24 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@5a2313f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
packages/davinci-client/src/lib/client.store.ts	0.00%	11 Missing ⚠️
packages/davinci-client/src/lib/collector.utils.ts	93.04%	8 Missing ⚠️
packages/davinci-client/src/lib/node.reducer.ts	84.21%	3 Missing ⚠️
packages/davinci-client/src/lib/davinci.api.ts	0.00%	1 Missing ⚠️
packages/davinci-client/src/lib/wellknown.api.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main       #3   +/-   ##
=======================================
  Coverage        ?   45.73%           
=======================================
  Files           ?       18           
  Lines           ?     2003           
  Branches        ?      123           
=======================================
  Hits            ?      916           
  Misses          ?     1087           
  Partials        ?        0

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

changeset-bot · 2025-01-10T21:10:58Z

🦋 Changeset detected

Latest commit: 8577a48

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@forgerock/davinci-client	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

cerebrl

I think there are some fairly major changes that need to happen here. I feel we shouldn't expose the element type to the developer. This is why each field should have an inputType key that just suggests something is SINGLE_SELECT or MULTI_SELECT. Whether it's a dropdown or radio doesn't really matter.

Let's talk about this in standup on Tuesday.

cerebrl · 2025-01-13T23:55:12Z

packages/davinci-client/src/lib/client.types.test-d.ts

Can we remove the -d from the filename? I would think types.test would be enough.

We can, however this is a convention of vite. https://vitest.dev/guide/testing-types

it specifically calls tsc under the hood, if we use test I think it may confuse how the file should be run/ executed.

cerebrl · 2025-01-14T00:05:46Z

packages/davinci-client/src/lib/collector.types.ts

+export type InferSingleValueCollectorFromSingleValueCollectorType<
+  T extends SingleValueCollectorTypes,
+> = T extends 'TextCollector'
+  ? TextCollector
+  : T extends 'DropDownCollector'
+    ? DropDownCollector
+    : T extends 'ComboboxCollector'
+      ? ComboboxCollector
+      : T extends 'RadioCollector'
+        ? RadioCollector
+        : //: T extends 'LabelCollector'
+          //? LabelCollector
+          T extends 'PasswordCollector'
+          ? PasswordCollector
+          : T extends 'FlowLinkCollector'
+            ? FlowLinkCollector
+            : /**
+               * At this point, we have not passed in a collector type
+               * or we have explicitly passed in 'SingleValueCollector'
+               * So we can return either a SingleValueCollector with value or without value
+               * or without a value.
+               **/
+              | SingleValueCollectorWithValue<'SingleValueCollector'>
+                | SingleValueCollectorNoValue<'SingleValueCollector'>;


I'm not sure what to think of this. Is this a recommended pattern that I'm just unfamiliar with, or is there some requirement we have that necessitates such a complex, deeply nested structure?

The reason i created this type was to better infer the type.

I definitely do not claim this to be the best way to do it, and if you feel there is not enough value derived from this, we can ignore it.

But i'd like to hear your thoughts outside of just purely this type, if you think the value it provides is worth the "complexity" of the type. (its just how we can write "if else" in types)

cerebrl · 2025-01-14T00:13:23Z

packages/davinci-client/src/lib/collector.utils.ts

+import {
+  type ActionCollectors,
+  type ActionCollectorTypes,
+  type InferSingleValueCollectorFromSingleValueCollectorType,
+  type SingleValueCollectorTypes,
 } from './collector.types';
-import type { DaVinciField } from './davinci.types';
+import type {
+  Combobox,
+  DaVinciField,
+  //LabelField,
+  Radio,
+  StandardFieldValue,
+} from './davinci.types';



It seems the two imports use different conventions even though they are both importing types. Can you change the first to import type {} from 'module', like you have the second import?

initial commit to start types for fields

adding support for form-field components

updating a few configurations in new repo for davinci app

adding a changeset and a few fixes to the repo for adding a changeset

adds types test files for many of the types files we have.

update pr comments and fix a few areas where type was wrong still

ryanbas21 · 2025-01-29T20:55:41Z

packages/davinci-client/src/lib/client.store.ts

@@ -118,6 +122,7 @@ export async function davinci({ config }: { config: DaVinciConfig }) {
        return function () {
          console.error('Collector not found');


obviously this was here, just want to make sure we want a console.error

Yeah, we should probably talk about this. Ultimately, I think it's good we log errors when they happen, but we probably need to add a control for this sometime soon (like logLevel). And, we should be consistent with it. I think some instances of errors we don't log with error.

ryanbas21 · 2025-01-29T21:05:28Z

packages/davinci-client/src/lib/davinci.types.ts

 };

-export type Radio = {
-  inputType: 'SingleSelect';
+export type SingleSelect = {


gotcha so these are "the same" type.

I wonder how this narrows, i'll read through a test.

I think it should narrow without a generic but my brain is questioning it

Not sure I understand your comment. Can you elaborate?

cerebrl

Other than the duplicate entry in .changeset/config.json, this looks good.

cerebrl · 2025-01-30T20:16:49Z

.changeset/config.json

  "linked": [],
  "access": "public",
  "baseBranch": "main",
  "updateInternalDependencies": "patch",
  "ignore": [
-    "@forgerock/*",
+    "@forgerock/mock-api-v2",


@ryanbas21 I'm sure you didn't intend to change this to something else?

cerebrl · 2025-01-30T20:22:45Z

packages/davinci-client/src/lib/client.store.ts

@@ -118,6 +122,7 @@ export async function davinci({ config }: { config: DaVinciConfig }) {
        return function () {
          console.error('Collector not found');


Yeah, we should probably talk about this. Ultimately, I think it's good we log errors when they happen, but we probably need to add a control for this sometime soon (like logLevel). And, we should be consistent with it. I think some instances of errors we don't log with error.

cerebrl · 2025-01-30T20:28:06Z

packages/davinci-client/src/lib/davinci.types.ts

 };

-export type Radio = {
-  inputType: 'SingleSelect';
+export type SingleSelect = {


Not sure I understand your comment. Can you elaborate?

ryanbas21 force-pushed the support-field-type branch 2 times, most recently from d5751bf to 4ed98c7 Compare January 10, 2025 21:10

ryanbas21 force-pushed the support-field-type branch 7 times, most recently from dd7aef7 to 0424867 Compare January 13, 2025 20:08

cerebrl requested changes Jan 14, 2025

View reviewed changes

ryanbas21 added 4 commits January 14, 2025 10:49

chore: start-types

9083f1a

initial commit to start types for fields

feat: add-form-field-components

16926bc

adding support for form-field components

chore: davinci-app-configurations

865e347

updating a few configurations in new repo for davinci app

chore: add-changeset

5fc1b35

adding a changeset and a few fixes to the repo for adding a changeset

ryanbas21 force-pushed the support-field-type branch from 0424867 to 81efc38 Compare January 14, 2025 17:50

test: add-types-tests

ae848c8

adds types test files for many of the types files we have.

ryanbas21 force-pushed the support-field-type branch from 81efc38 to ae848c8 Compare January 14, 2025 17:51

ryanbas21 and others added 2 commits January 14, 2025 11:22

chore: pr-comments

9aa0791

update pr comments and fix a few areas where type was wrong still

feat(davinci-client): refactor support for single select collectors

56e964d

cerebrl force-pushed the support-field-type branch from e451111 to 56e964d Compare January 28, 2025 17:34

chore: reset-deps

f26f1f7

ryanbas21 commented Jan 29, 2025

View reviewed changes

ryanbas21 force-pushed the support-field-type branch 2 times, most recently from 728520e to b785213 Compare January 29, 2025 22:39

chore: extensions-remove-unused

8577a48

ryanbas21 force-pushed the support-field-type branch from b785213 to 8577a48 Compare January 29, 2025 22:47

cerebrl approved these changes Jan 30, 2025

View reviewed changes

chore: update-changeset-config

ee4cb61

ryanbas21 merged commit ee8c7aa into main Jan 30, 2025
3 of 4 checks passed

ryanbas21 deleted the support-field-type branch January 30, 2025 21:09

github-actions bot mentioned this pull request Jan 30, 2025

Release PR #11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support field type #3

Support field type #3

ryanbas21 commented Jan 9, 2025

nx-cloud bot commented Jan 9, 2025 •

edited

Loading

codecov-commenter commented Jan 9, 2025 •

edited

Loading

changeset-bot bot commented Jan 10, 2025 •

edited

Loading

cerebrl left a comment

cerebrl Jan 13, 2025

ryanbas21 Jan 14, 2025

cerebrl Jan 14, 2025

ryanbas21 Jan 14, 2025

cerebrl Jan 14, 2025

ryanbas21 Jan 29, 2025

cerebrl Jan 30, 2025

ryanbas21 Jan 29, 2025

cerebrl Jan 30, 2025

cerebrl left a comment

cerebrl Jan 30, 2025

cerebrl Jan 30, 2025

cerebrl Jan 30, 2025

		@@ -118,6 +122,7 @@ export async function davinci({ config }: { config: DaVinciConfig }) {
		return function () {
		console.error('Collector not found');

Support field type #3

Support field type #3

Conversation

ryanbas21 commented Jan 9, 2025

JIRA Ticket

Description

nx-cloud bot commented Jan 9, 2025 • edited Loading

codecov-commenter commented Jan 9, 2025 • edited Loading

Codecov Report

changeset-bot bot commented Jan 10, 2025 • edited Loading

🦋 Changeset detected

cerebrl left a 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

Choose a reason for hiding this comment

cerebrl left a 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

nx-cloud bot commented Jan 9, 2025 •

edited

Loading

codecov-commenter commented Jan 9, 2025 •

edited

Loading

changeset-bot bot commented Jan 10, 2025 •

edited

Loading