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

Align Node server validation with Java server (async live validation) #99

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

martin-fleck-at
Copy link
Contributor

@martin-fleck-at martin-fleck-at commented Nov 26, 2024

What it does

See also eclipse-glsp/glsp-server#247
Fixes eclipse-glsp/glsp#1408

How to test

  • Add breakpoint during validation and see that the diagram still shows up with the 'Validating model...' message being present.

Follow-ups

Changelog

  • This PR should be mentioned in the changelog
  • This PR introduces a breaking change (if yes, provide more details below for the changelog and the migration guide)

Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general the changes look good to me.
However, I'm not the biggest fan of pulling in additional libraries just to use one function.
Especially if its something like debounce which we can easily implement ourself.
I would be in favor of replacing the loadash import with a custom debounce function.

@martin-fleck-at
Copy link
Contributor Author

@tortmayr I agree that pulling in libraries for a single function is not always the best option. However, we already use lodash in the client and in the theia-integration. Do you really think the impact is that large? I'm always a bit worried to re-implement such a common pattern. Note that we can already see the lodash dependency in the yarn.lock because we have concurrently in our dev (!) dependencies.

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

Successfully merging this pull request may close these issues.

Trigger live validation in async fashion
2 participants