Networks - Remove the third node

[xhulz]

Remove the third node from the Treeview and relocate the "Copy RPC endpoint address" option to the second node.

[acuarica]

This looks cool @xhulz! Should we implement this only for Ganache Service and Other Service? Because the Infura Service nodes have a completely different structure.

[xhulz]

Hey @acuarica!

Sure thing :)

[michaeljohnbennett]

yeah makes sense, was an odd leaf node, is there much other options/elements we would want to add to these menus. Not sure what other features/options we might want in the future.

[michaeljohnbennett]

I have been looking/refactoring the code to make this work and if we are to remove the LocalNetworkNode we need to work on the following assumptions/rules I guess:

• There will only be one port per local project name. I think is ok/normal as we can't really create more than LocalNetworkNode per LocalProject but the code is designed to have multiple.
• Debugger and some other code will be driven by this singular value, again I think this is going to be fine, its just the way the code was written to possibly handle multiple.

If we think this is ok then I can get finish my refactoring/cleanup

The most problematic code is here:

src/Models/TreeItems/LocalProject.ts:31

public async getDeployDestinations(): Promise<IDeployDestination[]> {
    const {local} = Constants.treeItemData.service;

    const getDeployName = (labelNode: string) => [local.prefix, this.label, labelNode].join('_');

    return Promise.all(
      (this.getChildren() as LocalNetworkNode[]).map(async (node) => {
        return {
          description: await node.getRPCAddress(),
          detail: local.label,
          getTruffleNetwork: async () => {
            const truffleNetwork = await node.getTruffleNetwork();
            truffleNetwork.name = getDeployName(node.label);
            return truffleNetwork;
          },
          label: getDeployName(node.label),
          networkId: node.networkId,
          networkType: node.itemType,
          port: node.port,
          options: this.options,
        } as IDeployDestination;
      })
    );
  }

This needs to be refactored completely.

Same here for debugging:

src/debugAdapter/debugNetwork.ts:127

private async getHost(projects: LocalProject[], providerUrl?: string): Promise<LocalNetworkNode> {
    if (providerUrl) {
      return projects
        .find((project) => {
          const network = project.getChildren().at(0) as LocalNetworkNode;
          const url = `${network.url.protocol}//${network.url.host}`;
          return url === providerUrl;
        })!
        .getChildren()
        .at(0) as LocalNetworkNode;
    } else {
      const items = projects.map((project) => {
        return {
          label: `$(debug-alt) ${project.label}`,
          detail: project.description,
        } as QuickPickItem;
      });

      const pick = await showQuickPick(items as QuickPickItem[], {
        placeHolder: Constants.placeholders.selectGanacheServer,
        ignoreFocusOut: true,
      });

      const project = projects.find((project) => project.description === pick.detail) as LocalProject;
      return project.getChildren().at(0) as LocalNetworkNode;
    }
  }

@acuarica @xhulz thoughts?

[michaeljohnbennett]

The other option is to keep the network nodes and just remove the display of them but that might be more complex than it seems with the way the factory that resolves the nodes for display.

[xhulz]

Yeah you are right @michaeljohnbennett. IMHO, the way this was done is very tight, and will need a lot of refactoring. Maybe for now, it would be better to keep it that way to have some time to think about the best structure for it

Thank you for that

[acuarica]

Hi @michaeljohnbennett, thanks for taking care of this. I don't have a clear idea of what kind of refactoring we are taking about here. Given said that, I'm more inclined to the refactor option if you feel like it. I believe we need to lean towards a more maintainable codebase.

[michaeljohnbennett]

I can do more on this, I think the networks themselves could be embedded into the main object better, it just seems the data model is really tied to the treeview model.

[xhulz]

Hey @michaeljohnbennett!

I agree 100% with you about embedding the network in the main object. I was talking to @acuarica about that and might be a great worth refactoring that just to try to untie this code. It will take time but in the end we can have a more flexible Treeview.

Thank you again!