fix: fix-the-url-type #509
Conversation
🦋 Changeset detectedLatest commit: 717e542 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 717e542. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
ryanbas21
force-pushed
the
davinci-client-fixes
branch
from
38b04fe to
51cfce1
Compare
ryanbas21
force-pushed
the
davinci-client-fixes
branch
from
51cfce1 to
c08ab2c
Compare
| type: string; | ||
| url: string | null; | ||
| }; | ||
| output: ActionCollectorOutputTypes; |
There was a problem hiding this comment.
I'm not entirely sure what we are doing here. This interface is named ActionCollectorWithUrl, but the output may or may not have a url. That seems a bit wonky.
May I ask why we are going to such lengths to avoid just having a single type that has url as an optional property? I've noticed this complexity, and I'm just not sure what value we get from it. But, I'm probably missing something.
ryanbas21
force-pushed
the
davinci-client-fixes
branch
from
c08ab2c to
ae0c074
Compare
| * DaVinci configuration options that extends the Forgerock SDK configuration options | ||
| */ | ||
| // export type InternalDaVinciConfig = | ||
| // | ((DavinciConfigWithResponseType | AsyncConfigOptions) & { | ||
| // | ((DaVinciConfigWithResponseType | AsyncConfigOptions) & { | ||
| // openIdConfiguration: OpenIdResponse; | ||
| // }) | ||
| // | (DavinciConfigWithResponseType | AsyncConfigOptions); | ||
| // | (DaVinciConfigWithResponseType | AsyncConfigOptions); |
There was a problem hiding this comment.
If we no longer need these, let's remove them. I commented them out as I wasn't sure if I was going to need them or not.
| | ActionCollector<'SocialLoginCollector'> | ||
| | ActionCollector<'ActionCollector'> | ||
| | ActionCollector<'FlowCollector'> | ||
| | ActionCollector<'SubmitCollector'>; |
There was a problem hiding this comment.
Let's make sure this and the associated types get reverted back to the previous state.
Type issue which i changed the structure of. This caused a few test failures because it is an api change
fixed action collector type to be more clear of intent
ryanbas21
force-pushed
the
davinci-client-fixes
branch
from
176c5e3 to
59605c2
Compare
Type issue which i changed the structure of. This caused a few test failures because it is an api change
JIRA Ticket
Please link jira ticket here
Description
Type of Change
Please Delete options that are not relevant
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Definition of Done
Check all that apply
Documentation