fix: remove extra parsing of lane elements which are not in the BPMN spec #2805
Conversation
|
♻️ PR Preview 1ed1609 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
|
♻️ PR Preview 1ed1609 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
- update unit tests for lane/laneSet - remove the configuration of TProcess type to set any string as property like : process[ShapeBpmnElementKind.LANE]
csouchet
force-pushed
the
2188-Remove_extra_parsing_of_lane_elements
branch
from
2912245 to
ee7e9ca
Compare
| // sometimes eventDefinition is simple and therefore it is parsed as empty string "", in that case eventDefinition will be converted to an empty object | ||
| const eventDefinitions: string | TEventDefinition | (string | TEventDefinition)[] = definitions[eventDefinitionKind + 'EventDefinition']; | ||
| // sometimes eventDefinition is simple, and therefore it is parsed as empty string "", in that case eventDefinition will be converted to an empty object | ||
| const eventDefinitions = definitions[(eventDefinitionKind + 'EventDefinition') as keyof TDefinitions] as EventDefinitions; |
There was a problem hiding this comment.
question: It is fine to introduce the new type as the code is now easier to read IMHO.
However, I don't understand why switching the syntax to "as"? I suggest to keep the existing it and to set an eslint rules to enforce this as part of #2742.
These are clearly extra changes. During my review, I had to wonder if this was part of the correction. So I guess if I or anyone else reads this in the future, the same question will arise.
There was a problem hiding this comment.
I do it, because is not working with the last syntax due to the remove of [key:string]:any 🙂
| @@ -70,9 +70,9 @@ const computeSubProcessKind = (processedSemanticType: BpmnSemanticType, bpmnElem | |||
| } | |||
| }; | |||
|
|
|||
| const orderedFlowNodeBpmnTypes: BpmnSemanticType[] = ['adHocSubProcess', 'transaction'] // specific management for adhoc and transaction sub-processes which are handled with a dedicated ShapeBpmnSubProcessKind | |||
| const orderedFlowNodeBpmnTypes: BpmnSemanticType[] = (['adHocSubProcess', 'transaction'] as BpmnSemanticType[]) // specific management for adhoc and transaction sub-processes which are handled with a dedicated ShapeBpmnSubProcessKind | |||
There was a problem hiding this comment.
question: Same question as in EventDefinitionConverter.ts about switching the syntax to "as".
We already have this discussion when I create the PR about this code, and we concluded that this was extra typing.
There was a problem hiding this comment.
Same answer, TypeScript send an error due to the removing of [key:string]:any 🙂
|
Kudos, SonarCloud Quality Gate passed! |
tbouffard
changed the title
tbouffard
changed the title
TProcessparsing: Removed unnecessary parsing oflaneelements of theTProcesstype that are not specified in the BPMN specification. This ensures correct parsing behavior.[key: string]: any;property has been removed from both theTProcessandTDefinitionsclasses. This prevents inadvertent access to non-existent properties within these objects.updateBpmnElementfunction within theJSONBuilderclass for better clarity. The function code has also been streamlined to improve readability and comprehension.Closes #2188