Update Serving API - from backend PR #6633 - #386
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the PlanStatus and Products types and refactors several goal and trigger definitions to use enum members instead of string literals. Feedback indicates that changing ConfigGoalBase.type from an array to a single GoalTypes value is overly restrictive and will cause TypeScript errors for specific goal types. Additionally, it is recommended to add 'visits_page' to the GoalTypes enum to ensure consistency across the NoSettingsGoal definition.
| /** | ||
| * List of goal types to be returned | ||
| */ | ||
| type?: Array<GoalTypes>; | ||
| type?: GoalTypes; |
There was a problem hiding this comment.
The change from Array<GoalTypes> to GoalTypes is too restrictive. Several goal types that extend ConfigGoalBase (such as GaGoal, SubmitsFormGoal, ClicksLinkGoal, and ClicksElementGoal) use type strings that are not present in the GoalTypes enum, which will lead to TypeScript errors. Additionally, the JSDoc comment 'List of goal types to be returned' should be updated to reflect that it is now a single value.
|
|
||
| export type NoSettingsGoal = ConfigGoalBase & { | ||
| type?: 'advanced' | 'visits_page' | 'code_trigger'; | ||
| type?: GoalTypes.ADVANCED | 'visits_page' | GoalTypes.CODE_TRIGGER; |
There was a problem hiding this comment.
This line mixes enum members (GoalTypes.ADVANCED, GoalTypes.CODE_TRIGGER) with a string literal ('visits_page'). For consistency and better type safety, visits_page should be added to the GoalTypes enum and used here.
2 participants
Updating TS Serving API after the latest changes from backend repo,
PR #6633