-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Fix Run API test type warnings #2343
Conversation
Removed vultr server and associated DNS entries |
e080d0e
to
ac8ccca
Compare
?.map(([_nodeId, nodeData]) => nodeData?.data?.allowInviteToPay); | ||
const inviteToPayEnabled = (flowGraph: FlowGraph): boolean => { | ||
const payNodes = Object.entries(flowGraph).filter( | ||
(entry): entry is [string, Node] => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type narrowing here has to be repeated as it doesn't propagate as expected from the function which is being called.
type: ComponentType, | ||
): entry is [string, Node] => { | ||
const [nodeId, node] = entry; | ||
if (nodeId === "_root") return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative (and possibly better) solution to this would be to add type property to the root node (ComponentType._Root
?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these tidy ups! I can try to revisit those lingering Digital Land ones soon - I think it's just a matter of typing the shape the expected response from their Entity & Metadata endpoints.
export interface Node { | ||
id?: string; | ||
data?: Record<string, any>; | ||
edges?: Array<string>; | ||
type?: number; | ||
} | ||
|
||
/** | ||
* @deprecated Migrating to Flow and FlowGraph from planx-core | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 these are helpful reminders, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep - hopefully it just means when we next need to reach for a type we avoid these ones 👌
What does this PR do?
planx-core
I've been staring at the message below a fair amount these past few days trying to sort E2E tests, so this is just a quick pass at addressing a few of these issues.
Next steps...
The eventual aim would be to remove the Passport / Flow etc types from the API and just use the ones from
planx-core
. This isn't a small change though so I'll just try chipping away at them here and there as a maintenance task 👍