Skip to content
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

Add FEEL errors to linting #22

Closed
4 tasks done
marstamm opened this issue Sep 27, 2022 · 4 comments
Closed
4 tasks done

Add FEEL errors to linting #22

marstamm opened this issue Sep 27, 2022 · 4 comments
Assignees

Comments

@marstamm
Copy link
Member

marstamm commented Sep 27, 2022

Epic: https://github.com/camunda/product-hub/issues/280

Follow-ups:
bpmn-io/properties-panel#190

@marstamm marstamm added the in progress Currently worked on label Sep 27, 2022
@marstamm marstamm self-assigned this Sep 27, 2022
@CatalinaMoisuc
Copy link
Member

CatalinaMoisuc commented Oct 18, 2022

@marstamm please open an integration issue in the Web Modeler repository.

As discussed, maybe just a version bump is enough.

@marstamm
Copy link
Member Author

Performance

  • Adding FEEL expression Linting adds ~0.1 ms linting time per expression
  • Because linting is done synchronously, this can cause lag when modeling

When tested with a 2.000 expression diagram, the lag was noticable but still usable. I think the performance impact is still small enough that we can open the base PRs without performance improvements.

Still, I would like to look into into caching now, so we have a solid foundation when we want to introduce Variable null checks and other more resource intensive linting rules

@marstamm
Copy link
Member Author

Caching Spike results

Caching creates the challenge of creating hashes for the Objects, so we can figure out if they changed in the next run. We need to do this not only for the Element itself but for the complete subtree, as some lint rules access extension elements while others are using the child elements directly.

While I was able to build working caching, the performance is worse than running linting normally and adds time for every element.

Instead of caching, we could think about linting changes only, e.g. by integrating into eventBus events. This would require a bigger refactoring, so I will not pursue it further now.

@marstamm
Copy link
Member Author

marstamm commented Nov 8, 2022

done via #31

@marstamm marstamm closed this as completed Nov 8, 2022
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants