-
Notifications
You must be signed in to change notification settings - Fork 33
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
[INFRA] Activate noImplicitAny
compiler option
#460
Conversation
bd182f1
to
81267fb
Compare
noImplicitAny
compiler option
81267fb
to
5ab41d1
Compare
48b755d
to
c27f8f2
Compare
c27f8f2
to
dc06629
Compare
} else if (kind === ShapeBpmnElementKind.TEXT_ANNOTATION) { | ||
const name = 'text' in bpmnElement ? bpmnElement.text.toString() : undefined; | ||
shapeBpmnElement = new ShapeBpmnElement(bpmnElement.id, name, kind, processId); | ||
} else { | ||
const name = kind === ShapeBpmnElementKind.TEXT_ANNOTATION ? bpmnElement.text : bpmnElement.name; | ||
shapeBpmnElement = new ShapeBpmnElement(bpmnElement.id, name, kind, processId, bpmnElement.instantiate); | ||
const name = 'name' in bpmnElement ? bpmnElement.name : undefined; | ||
const instantiate = 'instantiate' in bpmnElement ? bpmnElement.instantiate : undefined; | ||
shapeBpmnElement = new ShapeBpmnElement(bpmnElement.id, name, kind, processId, instantiate); |
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.
❓ I don't fully understand why this change is needed?
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.
If we don't use any as type, there is an error, because all the types don't have all the properties.
So, we can keep as it was before, or we can have it, or we can separate all flownode convertions in dedicated converters.
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.
ok I understand now, I have also done some test locally to see what kind of errors the TS compiler generates.
I don't think we gain something on the code readability here, the code is now harder to read now IMHO
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.
ok with the latest changes. As discussed with @csouchet we agreed to add @ts-ignore
to avoid complex code structure (check if a property exists) when we know it exists for sure and tests exist
Covers #421
Already activated, but I remove some ignoring comments