-
Notifications
You must be signed in to change notification settings - Fork 195
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
Feature request: allow multiple paths for packageDefaultDependenciesDirectory
#149
Comments
Hi @PaulRBerg, Obviously order of preference is important.. and you could end up using dappsys libraries with zeppelin. I will do some experimentation and see how it goes. |
Thank you @juanfranblanco 😊 |
Hi @juanfranblanco , just my thoughts on this: Truffle resolves the import dependencies as it should be, how hard is to copy the module resolution mechanism from there? In my opinion a consistent solution should be using the same resolution system that truffle or any other linter uses to resolve the imports in the target file Or as @PaulRBerg proposes, an array of paths, but that will still be too much constrained to specific projects... Another solution would be to allow using The main issue is that nowadays mono-repo project structures are quite common, and those repos normally have a set of dependencies installed directly into the root I tried to struggle and change some code in the extension myself, should it be modified into |
The paths need to support also the DappHub / Dappsys structure as used by Maker, so the multiple path array could solve the problem. https://github.com/dapphub |
Although as we have seen in DappSys, we have a path + contractFolder, hence there are two settings. So it is going to need an array of these objects containing both values. Other thought is that imports will need to be resolved before hand, ie do they exists before rewriting them. Many years ago Christian (solidity) and I discuss the pattern. Which mainly for Libraries the import does not start with any ".", this indicates that these need to be resolved using the library mechanism. |
Is this resolved yet? My imports are all flagging as not found, despite existing and compiling, resulting not linting or other error detection (because this error is a show stopper for processing the rest of the file). |
Truffle's path resolver is actually pretty modular and published as a separate npm package:
Maybe it would be possible to just use it as a dependency? I think it might even accomodate the dappsys use case. For example handling of |
@cameel It will be much easier to extend it to use array paths for dependencies than adding a dependency to truffle. As usual making sure that it does not break current deployments. |
Edit: And yes I have not got around to do it, as most people just use node_modules as the main dependency folder. |
Yeah, agreed on that, a path list is definitely simpler and easier to maintain than a full-blown resolver. The downside though is that with some less common project layouts (like Incidentally, we'll soon be adding a similar feature in the Solidity compiler and we went for a path list solution too: ethereum/solidity#11409. In our case it's motivated by not wanting to make too many assumptions about how projects are structured so there won't even be any defaults other than the working directory. The compiler itself should be agnostic about that. BTW, for anyone wondering about this, the compiler feature is only for command-line usage with an import callback so unfortunately it will not be of any help here - this extension uses Standard JSON input without a callback.
Just wanted to add some nuance here since I've been digging into this exact topic recently :) That's generally true but only because the ecosystem is close to a monoculture now in that regard. People use almost exclusively JS-based tools (i.e Truffle or Hardhat) but there are also some less popular tools that do not rely on npm and thus do not use |
I played around with Solidity and did some Hardhat example projects, effectively ending up with a mono-repo. I got the zeppelin import error, but the worst part was that the error didn't help much. It just told me that it couldn't find the import. Searching online didn't lead here. I tried multiple configurations for the contract/node_modules directory but couldn't get it to work. In a chat, someone mentioned that "it's sad the vscode solidity plugin doesn't work with mono-repos" when I finally found my problem. It would be cool if mono-repos would work directly, or at least the errors would be a bit more insightful. Anyway, excellent extension, thanks for it!! |
hi @kay-is and all, You can use "remappings" now that will allow you to support multiple directories and specific mappings Please let me know how it goes. Edit: Also as you have mono-repos, now vscode supports having multiple "virtual" folders in a workspace, each one will allow you to add any folder in a workspace, and each folder can have their own "remapping.txt" file. |
@kay-is if this does not work.. let me know! |
@juanfranblanco when I have a structure like that:
How would I define the correct mappings? |
Ah, in that scenario the best thing will be to create (at the moment) a workspace and add each folder to the workspace. Each folder (greeter, fundraiser) having their own "remappings.txt" file. Although as each folder is just using "node_modules" by adding each folder to the workspace, each folder will be considered a root, so will be using "node_modules" as a folder without having to add a "remappings.txt" file. Now, in the future if, we have a project file as a standard, where this file is located that could be considered the root, so I was thinking of using the "remappings.txt" as such. |
I think this might be a good idea not only for mono-repos. How would one go about and set up the dependencies folders for globally installed packages? In my case I have truffle installed globally while other packages like openzeppelin locally. |
@kerloom for this you can use remappings, Edit: this is what it is used by brownie Edit2: You can put your remappings in the settings also. |
@kerloom there is also now specific remapping settings for unix / windows so these can be shared globally in a specific location, working directory |
@juanfranblanco how would you do a project structured like this with yarn workspaces:
I am using remappings.txt inside the foundry file. Still getting "File import callback not supported" Perhaps I'm just doing it the hard way and need a different structure. Thanks! |
@carllippert the remappints.txt needs to be set in the root folder. Let me know how it goes. |
I can currently set the location of "node_modules" like this:
The structure of my repo looks like this:
Since I'm using yarn workspaces, most of my third-party contracts are cached at the root. vscode-solidity has no trouble with those contracts.
However, sometimes I have to use the nohoist feature, which moves the contracts down below in "package1", "package2", etc. When that happens, I get this error:
This could be solved by allowing multiple (glob) paths in the
packageDefaultDependenciesDirectory
user setting. Example:The text was updated successfully, but these errors were encountered: