Feature/command provider poc #204

michaeljohnbennett · 2022-08-15T09:42:14Z

This is the intial POC for the concept of alternative providers for the main commands on a project.

I spent a bit of time to and fro on how best to achieve or abstract out the concept of a workspace and a provider and this is the initial version where we have providers that can interrogate the workspaces and decide what provider is where in a monorepo for ones we support.

Still early days but I think it could be adapted to give us some flexibility in supporting other frameworks easily in the future.

Going to mark this draft right now as I'd like to get your thoughts on the thinking behind it and how we might be able to do it better/enhance it.

Things we could improve:

having config and commands driven by the folder or parent folder config.
understanding what can and cannot be done in each project and how things like package.json commands/sdk commands may interoperate or conflict.

Add in POC to test other providers under the hood.

Lot of fuffing around to figure out what to mock when doing a test.

…ovider-poc

Removing unused config elements also.

More refactoring and adding HH component finder.

…ovider-poc

This is the 3rd attempt.

Output channel has a wrapper to make logging easier.

…ovider-poc

Fixed the NPM and code which found/resolved the paths for projects into an abstract resolver. Hasn't been merged over the truffle code write now but put into a seperate namespace to keep it isolated and for comment.

acuarica

Thanks @michaeljohnbennett for working on this. This would be a cool addition. Totally agree, with your points.

src/Constants.ts

package.json

src/helpers/required.ts

tsconfig.json

typings/global/index.d.ts

acuarica · 2022-08-15T20:57:17Z

src/helpers/workspace.ts

@@ -185,3 +189,120 @@ async function selectTruffleConfigFromQuickPick(workspaces: TruffleWorkspace[]):

  return result.truffleWorkspace;
 }
+
+export namespace AbstractWorkspaceManager {


I believe I would put all HH related stuff under a module HH. Or even as part of the extension adapter. Otherwise I won't be maintainable, say if we add more SDK providers.

It is abstract.

It is provider agnostic hence the name. There are multiple resolvers inside it so in theory it would remove all the other code above in time and make the resolving of config and uri locations agnostic. so if we find a truffle config in folder A, then folder A is a truffle based SDK command driven workspace/folder. Folder B could hh or something else?

Does that make sense?

Namespace was here to just encapsulate it in this file v's making another workspace file.

xhulz

Hey @michaeljohnbennett,

It looks awesome :)

I agree with @acuarica comments. I'd like to point that we can keep the output wrapper, but I think might be a good idea, raise another issue to change the other output callings on the code, just to keep the pattern

Thank you for that :)

acuarica · 2022-08-17T13:33:36Z

src/services/extensionAdapter/HardHatExtensionAdapter.ts

+import {NotificationOptions} from '@/Constants';
+import {showNotification} from '@/helpers/userInteraction';
+import {IExtensionAdapter} from '@/services/extensionAdapter/IExtensionAdapter';
+import {Constants} from '@/constants';


Here we should use the same casing, i.e., @/Constants. Otherwise, webpack gives a warning

There are multiple modules with names that only differ in casing. This can lead to unexpected behavior when compiling on a filesystem with other case-semantic.

removed some dead imports and lowered the hardhat min version.

Added in methods to make folder based ExtensionMapping work. Still an odd issue that the picker per project is happening twice but will be easy enough to figure out whats going on.

michaeljohnbennett · 2022-11-01T07:17:43Z

@acuarica @xhulz I have made more fixes and now the actual resolving of providers is dynamic based on the folders it finds and configs inside them.

Still a bit to fix but overall its working for compilation. But we need to figure out the display in the contracts folders.

acuarica · 2022-11-01T19:28:04Z

Hi @michaeljohnbennett, maybe we could use the approach I've used for deployments. Whenever more than one Truffle is found, the Deployments view uses the folder name+Truffle config name as a way to identify each project, for example

This was implemented in #179.

michaeljohnbennett · 2022-11-01T21:52:02Z

Hi @michaeljohnbennett, maybe we could use the approach I've used for deployments. Whenever more than one Truffle is found, the Deployments view uses the folder name+Truffle config name as a way to identify each project, for example

This was implemented in #179.

I was trying to figure out a easy way to do this with the current treeview for file we use in that contract window, I'll have a look at the deployments view code again and see if I can adapt it. I was thinking even a logo against the folders or something would be good. 👍🏻

moving the code to be more workspace aware. adding code to handle workspaces that have no/unknown frameworks installed so they fail gracefully. TODO: somehow add in a notification on UnknownExtensionAdapter.ts to warn user their commands are benign.

update to latest sinon (was very old) refactored out the old TruffleWorkspace code and move everything to AbstractWorkspace aware. workspace.ts removed and refactored to a Helper class WorkspaceHelpers .ts which now has all the base workspace helpers that are more VSCode specific. Core commands and TruffleCommands.ts are all AbstractWorkspace aware now as well. Tests cleaned up. A few placeholders added to wire out tomorrow.

acuarica · 2022-11-15T13:31:10Z

src/helpers/AbstractWorkspace.ts

+ * The [glob](https://github.com/isaacs/node-glob#glob-primer) pattern to match Truffle/Other config file names.
+ */
+export const TRUFFLE_CONFIG_GLOB = 'truffle-config{,.*}.js';
+export const HARDHAT_CONFIG_GLOB = 'hardhat.config{,.*}.ts';


We would also need to include Hardhat js config files, i.e., we can use the following glob hardhat.config{,.*}.{js,ts}.

ok I'll make that change I just think the majority of HH projects are TS based. I don't think they even give you a choice when bootstrapping a project to use JS anymore?

As of HardHat v2.12.2 they do

I will change the glob. thanks! 👍🏻

acuarica · 2022-11-15T15:00:20Z

Hey @michaeljohnbennett, doing some early testing as discussed earlier.

The Contract Explorer now displays the whole workspace, instead of displaying the contracts folder only. This happens for both Truffle and Hardhat projects.

acuarica · 2022-11-15T15:22:00Z

webpack.common.js

+// Copyright (c) 2022. Consensys Software Inc. All rights reserved.
+// Licensed under the MIT license.
+


Thanks for taking care of the Copyright Notice. But given that it is a significant change, should we open a different PR for this?

I think this was done by my IDE. I thought this was how all our JS files were marked? Intellij adds in copyrights based on your template by default on all unmarked files.

I can revert all this if you think best?

Got it. If it not a big issue reverting, I believe it's better to create another PR to tackle this copyright thing.

michaeljohnbennett · 2022-11-15T22:07:34Z

Hey @michaeljohnbennett, doing some early testing as discussed earlier.

The Contract Explorer now displays the whole workspace, instead of displaying the contracts folder only. This happens for both Truffle and Hardhat projects.

Yes, this one of the questions I have. Contracts is really the wrong name going forward as we really need to show a bit more than just the contracts folder now, its a hierarchy of projects and their resulting folders.

Perhaps we need to have a filter list:

contracts (wherever that folder is)
tests
config files

?

michaeljohnbennett · 2022-11-15T22:24:54Z

I'll revert the copy right stuff and merge in develop today.

acuarica · 2022-11-15T23:09:59Z

Regarding #204 (comment), the only thing that worries me is that if we start including tests and config files into the Contract Explorer, not sure how much value provides over the File Explorer.

michaeljohnbennett · 2022-11-16T01:57:40Z

Regarding #204 (comment), the only thing that worries me is that if we start including tests and config files into the Contract Explorer, not sure how much value provides over the File Explorer.

I agree it needs to change but to what?

Stay as is and only show contracts, but how do we represent a monorepo of differing projects in this view? A folder for each root then a contracts folder? What about scripts/tests etc?
We change the name to something more akin to a Project explorer and show more meaningful information/folders relating to a smart contract project (tests, contracts, artifacts, config) but filter out all the other non-essential to coding files (.ignore etc, build/node_modules)

Add in chai as promised to help with promise testing. Fixed the tests around workspace resolving and got good coverage on all paths.

michaeljohnbennett · 2022-11-16T06:16:03Z

I have merged in develop and @xhulz fixes to tree view. I'll use them now as my changes I made are blasted away now but its ok, I still have to remove the copy right changes.

All tests fixed now as well.

acuarica · 2022-11-16T14:19:13Z

I agree it needs to change but to what?

I'm leaning towards option 1. described in #204 (comment).

but how do we represent a monorepo of differing projects in this view? A folder for each root then a contracts folder?

That's the current behavior, but the contracts folder is the root itself. For example in a workspace with two Truffle config files

each project is shown separately with its own root

What about scripts/tests etc?

This seems to be an orthogonal feature, not necessarily related to this PR. Given said that, I do agree that we need to discuss what to display in this view.

Stopped updating copyright automagically.

Fixing tests and optimising the code before I can do the next part which is abstract out the truffle resolver code for config etc.

Reworking the mocks to work in the VSCode integration test environment.

The joys...

michaeljohnbennett · 2022-11-29T22:52:05Z

marking as draft so it doesn't get inadvertently merged right now pre-release of latest version.

michaeljohnbennett added 10 commits July 26, 2022 14:26

feat: command provider poc

001c69d

Add in POC to test other providers under the hood.

fix: adding in tests and typings to make chai/mocha work

0ed7d89

Lot of fuffing around to figure out what to mock when doing a test.

Merge remote-tracking branch 'origin/develop' into feature/command-pr…

dabb72a

…ovider-poc

fix: adjusting provider

af4eaaa

Removing unused config elements also.

fix: adjusting provider

e8c58b6

More refactoring and adding HH component finder.

Merge remote-tracking branch 'origin/develop' into feature/command-pr…

8126c63

…ovider-poc

fix: rework again after merging

2e804e2

This is the 3rd attempt.

fix: added logging wrapper

27f20a7

Output channel has a wrapper to make logging easier.

Merge remote-tracking branch 'origin/develop' into feature/command-pr…

b13ae39

…ovider-poc

fix: added in compiler

b8904b6

Fixed the NPM and code which found/resolved the paths for projects into an abstract resolver. Hasn't been merged over the truffle code write now but put into a seperate namespace to keep it isolated and for comment.

michaeljohnbennett added the enhancement New feature or request label Aug 15, 2022

michaeljohnbennett requested review from kevinbluer, acuarica and xhulz August 15, 2022 09:42

michaeljohnbennett self-assigned this Aug 15, 2022

acuarica reviewed Aug 15, 2022

View reviewed changes

xhulz reviewed Aug 16, 2022

View reviewed changes

acuarica reviewed Aug 17, 2022

View reviewed changes

michaeljohnbennett added 4 commits August 24, 2022 11:08

fix: cleanup comments

18b9882

removed some dead imports and lowered the hardhat min version.

fix: merging in changes from develop

5d84c97

fix: logging fixed

ecd92be

fix: project based compilation working

e002cb6

Added in methods to make folder based ExtensionMapping work. Still an odd issue that the picker per project is happening twice but will be easy enough to figure out whats going on.

michaeljohnbennett added 3 commits November 7, 2022 18:32

fix: cleaning up code

b8f3da9

fix: lots of refactoring

9ae11c7

moving the code to be more workspace aware. adding code to handle workspaces that have no/unknown frameworks installed so they fail gracefully. TODO: somehow add in a notification on UnknownExtensionAdapter.ts to warn user their commands are benign.

acuarica reviewed Nov 15, 2022

View reviewed changes

michaeljohnbennett added 2 commits November 16, 2022 16:35

fix: tests

3c01bfe

Add in chai as promised to help with promise testing. Fixed the tests around workspace resolving and got good coverage on all paths.

Merge branch 'develop' into feature/command-provider-poc

90a7f12

michaeljohnbennett added 4 commits November 17, 2022 14:12

chore: removing copyright cleanup

7bfb7fe

Stopped updating copyright automagically.

chore: post merge refactor/cleanup

75e4935

Fixing tests and optimising the code before I can do the next part which is abstract out the truffle resolver code for config etc.

fix: broken integration tests

19a3be6

Reworking the mocks to work in the VSCode integration test environment.

fix: windows paths on CICD

5ed78f4

The joys...

michaeljohnbennett marked this pull request as ready for review November 21, 2022 05:55

Merge branch 'develop' into feature/command-provider-poc

3addbe8

michaeljohnbennett marked this pull request as draft November 29, 2022 22:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/command provider poc #204

Feature/command provider poc #204

michaeljohnbennett commented Aug 15, 2022

acuarica left a comment

acuarica Aug 15, 2022

michaeljohnbennett Aug 15, 2022

xhulz left a comment

acuarica Aug 17, 2022

michaeljohnbennett commented Nov 1, 2022

acuarica commented Nov 1, 2022 •

edited

michaeljohnbennett commented Nov 1, 2022

acuarica Nov 15, 2022

michaeljohnbennett Nov 15, 2022

acuarica Nov 15, 2022

michaeljohnbennett Nov 16, 2022

acuarica commented Nov 15, 2022

acuarica Nov 15, 2022

michaeljohnbennett Nov 15, 2022

acuarica Nov 15, 2022

michaeljohnbennett commented Nov 15, 2022

michaeljohnbennett commented Nov 15, 2022

acuarica commented Nov 15, 2022

michaeljohnbennett commented Nov 16, 2022

michaeljohnbennett commented Nov 16, 2022

acuarica commented Nov 16, 2022

michaeljohnbennett commented Nov 29, 2022

		// Copyright (c) 2022. Consensys Software Inc. All rights reserved.
		// Licensed under the MIT license.

Feature/command provider poc #204

Are you sure you want to change the base?

Feature/command provider poc #204

Conversation

michaeljohnbennett commented Aug 15, 2022

acuarica left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xhulz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaeljohnbennett commented Nov 1, 2022

acuarica commented Nov 1, 2022 • edited

michaeljohnbennett commented Nov 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acuarica commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaeljohnbennett commented Nov 15, 2022

michaeljohnbennett commented Nov 15, 2022

acuarica commented Nov 15, 2022

michaeljohnbennett commented Nov 16, 2022

michaeljohnbennett commented Nov 16, 2022

acuarica commented Nov 16, 2022

michaeljohnbennett commented Nov 29, 2022

acuarica commented Nov 1, 2022 •

edited