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

Refactor commands #550

Open
abelzis opened this issue Mar 5, 2024 · 0 comments
Open

Refactor commands #550

abelzis opened this issue Mar 5, 2024 · 0 comments
Labels
tech debt Technical debt

Comments

@abelzis
Copy link
Collaborator

abelzis commented Mar 5, 2024

Currently, there is an issue with Commands and it builds technical debt. Take a look at this example:

// query.tsx
function makeQuery() {
    const command: ICommand = {
        id: "Query",
        action: CommandAction.Query,
    }
    vscode.postMessage(command);
}

Notice here, that the command takes in id and action. The ComandAction is an enum of integers: CommandAction.Query = 2.

Then take a look at QueryEditor.ts:

this.panel.webview.onDidReceiveMessage((command: ICommand) => {
    switch (command.action) {
        case CommandAction.Query:
            const obj = {
                id: command.id,
                command: 'data',
           };
           this.panel?.webview.postMessage(obj);
}

For some reason, the webview posts message any object with id the reused id from the command variable. In this example, it's "Query". Then there is a property command. Why not action? And why is there no enum and a literal used? Appears to be inconsistencies.

For this ticket: Standardize the command interfaces and update all present commands.

  1. Move the ICommand interface and CommandAction enum to src/common/commands/commands.ts. Update the module imports. Remove the content?, params?, columns? properties. action property should be of type string.
  2. Create new file for each of the domains, e.g. for fields: src/common/commands/fieldsCommands.ts, for query queryCommands.ts, connection connectionCommands.ts etc. Then create the corresponding CommandAction interfaces for the domains. For example:
export enum ConnectionActionEnum {
   Save = "save",
}

export interface ConnectionSaveCommand inherits Omit<ICommand, 'action'> { 
   action: ConnectionActionEnum.Save;
   connectionConfig: IConfig;
}

Now this ConnectionSaveCommand can be used as an interface from either frontend and backend.
Note: It is possible, that FE -> BE can request one set of data and BE -> FE can use another set of data. In this case - separate Commands and CommandActions should be created.
3. Now whenever the event message is received, it should be of ICommand interface. Then, in the switch(command.action) you can use the specific CommandAction cases. When you need the exact values, just use according to this example: (command as ConnectionSaveCommand) or const saveCommand = command as ConnectionSaveCommand.
Example:

this._view?.webview.onDidReceiveMessage((command: ICommand) => {
    switch (command.action) {
        case ConnectionActionEnum.Save:
            const saveCommand = command as ConnectionSaveCommand;
            // do something
            break;
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Technical debt
Projects
None yet
Development

No branches or pull requests

1 participant