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

GLSP-1166 Fix dependency injection cycle in selection service #305

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

tortmayr
Copy link
Contributor

@tortmayr tortmayr commented Nov 29, 2023

Avoid binding of SelectionService as IModelRootListner. Instead directly inject theCommandStack into SelectionService and manually register itself as IGModelRootListener. This avoids a circular dependency injection if any of the SelectionListners wants to inject the ActionDispatcher.

Can be tested e.g. by creating a SelectionListener that injects the action dispatcher:

@injectable()
export class MySelectionListener implements ISelectionListener {
    @inject(TYPES.IActionDispatcher)
    protected actionDispatcher: ActionDispatcher;

    selectionChanged(root: Readonly<GModelRoot>, selectedElements: string[], deselectedElements?: string[] | undefined): void {
        console.log('Selection changed: ' + selectedElements);
    }
}

// bind it in the workflow diagram module

bindAsService(context, TYPES.ISelectionListener, MySelectionListener);

This fails on master with a circular dependency error but works with this change.

Fixes eclipse-glsp/glsp/issues/1166

Avoid binding of SelectionService as `IModelRootListner`. Instead directly inject the`CommandStack` into `SelectionService`
and manually register itself as `IGModelRootListener`.
This avoids a circular dependency injection if any of the `SelectionListner`s wants to inject the `ActionDispatcher`.
Copy link
Contributor

@martin-fleck-at martin-fleck-at left a comment

Choose a reason for hiding this comment

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

LGTM!

@tortmayr tortmayr merged commit 2247067 into master Nov 29, 2023
5 checks passed
@tortmayr tortmayr deleted the glsp-1166 branch November 29, 2023 08:23
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
…e-glsp#305)

Avoid binding of SelectionService as `IModelRootListner`. Instead directly inject the`CommandStack` into `SelectionService`
and manually register itself as `IGModelRootListener`.
This avoids a circular dependency injection if any of the `SelectionListner`s wants to inject the `ActionDispatcher`.

Fixes eclipse-glsp/glsp/issues/1166
holkerveen pushed a commit to holkerveen/glsp-client that referenced this pull request Dec 21, 2024
…e-glsp#305)

Avoid binding of SelectionService as `IModelRootListner`. Instead directly inject the`CommandStack` into `SelectionService`
and manually register itself as `IGModelRootListener`.
This avoids a circular dependency injection if any of the `SelectionListner`s wants to inject the `ActionDispatcher`.

Fixes eclipse-glsp/glsp/issues/1166
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActionDispatcher can not be injected into GLSPSelectionDataService implementations
2 participants