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

Extension considers sol files not part of hardhat project if project path contains symlink #270

Open
SAPikachu opened this issue Oct 11, 2022 · 9 comments
Labels
status:needs-more-info type:bug Something isn't working

Comments

@SAPikachu
Copy link

I use symlink to point my development workspace to a synced folder. When I open a hardhat project inside the symlink, I got following error and intellisense doesn't work correctly:

image

If I open the same project directly inside the real path without symlink, intellisense works correctly.

Environment:

VSCode (note that I use SSH remote extension to work with project inside a VM):

Version: 1.72.0 (user setup)
Commit: 64bbfbf67ada9953918d72e1df2f4d8e537d340e
Date: 2022-10-04T23:20:39.912Z (6 days ago)
Electron: 19.0.17
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Windows_NT x64 10.0.19044
Sandboxed: No

VM:

  • Ubuntu 20.04.5 LTS
  • Node v16.17.1
@kanej kanej added the type:bug Something isn't working label Oct 24, 2022
@fubhy
Copy link
Contributor

fubhy commented Dec 2, 2022

This also happens under certain circumstances (not yet entirely sure how to reproduce this reliably) when using pnpm with symlinked node modules that host smart contracts (e.g. openzeppelin, etc.)

@antico5
Copy link
Collaborator

antico5 commented Dec 7, 2022

@fubhy About the foundry + pnpm issue (copying my answer from slack):

I've been investigating the symlink issue using the repo you shared (https://github.com/KedziaPawel/symlink-hardhat-bug). I've arrived to 2 conclusions:

  • Using solidity >= 0.8.8 will solve the problem with our current released version of the extension (open zeppelin contracts version would have to be updated to be compatible)
  • Using the given versions (solidity < 0.8.8), you'd need to wait for our next release of the extension (0.6.4), which has a fix for base-path relative imports, and just then a workaround would be to change the imports from @openzeppelin/... to node_modules/@openzeppelin/...

Forge works on the provided settings because it uses the solc CLI instead of the json interface (which the extension uses).

I tried the following json inputs for solidity < 0.8.8 to try and make it work without success:
Given import path is import '@openzeppelin/...'
a) Set remappings to @OpenZeppelin => node_modules/@openzeppelin/
b) Set remappings to @OpenZeppelin => /absolute/path/to/node_modules/@openzeppelin/

Both give the "file outside allowed directories" error.

@SAPikachu I was able to reproduce the reported issue. Our extension does not behave as expected on hardhat project directories that are symlinked, because of the way we load the HRE for providing validation and other services (require.resolve, which unavoidably resolves symlinks). We will have to think about possible workarounds for this.

@fvictorio
Copy link
Member

This happens in fresh MUD projects too. Repro:

pnpm create mud@canary app
# selected vanilla, but i'm pretty sure it's not relevant
code app

And go to packages/contracts/src/systems/IncrementSystem.sol. The @latticexyz/world/src/System.sol import will show as not found.

@kanej kanej moved this to Todo in hardhat-vscode May 25, 2023
@kanej
Copy link
Member

kanej commented May 25, 2023

@alcuadrado suggested we investigate the library: https://www.npmjs.com/package/resolve, and whether it can help us with out node require resolution for pnpm.

@antico5 antico5 moved this from Todo to In Progress in hardhat-vscode May 31, 2023
@antico5
Copy link
Collaborator

antico5 commented May 31, 2023

@kanej I wasn't able to crack it so far. These are my findings on the issue:

  • Using the resolve package helps bypassing the natural resolution of symlinks by node (have to be explicit with preserveSymlinks option). Although this alone isn't enough to support symlinked projects.
  • When spawning the child processes for hardhat projects (workers), passing the cwd doesn't work for symlinked directories. Child process will be spawned with the cwd set to the resolved path.
  • Trying to work around this by passing the symlinked path e.g. by env variable, doesn't work either. I didn't find a way to change directories from a node process into a directory that is symlinked. Using process.chdir() will resolve the directory and the working directory will be set to the resolved path.
  • My next attempt was just avoiding any usage of cwd from the worker processes, and replacing them with the variable passed down on child spawning (which contains the symlink). This, using resolve, allows me to get the symlinked path to the hardhat module. But even though we require the files to get the HRE with the symlinked path, it seems that hardhat internally will resolve the symlink paths. Clues to this include that the directories exposed by the config (i.e. hre.config.paths.sources), and the dependency graph built by hardhat itself, expose paths with the resolved directories.
  • This mismatch of the file uris handled by the editor (symlinked paths), and paths handled by the framework (resolved), makes it so files are not recognized as part of the project, dependencies not able to be resolved, among other unknown issues.
  • A potential solution would be to add a conversion layer where all file uris used by the language client on requests (and also while globbing), are converted to their resolved version, so they would match once they are passed down to the hardhat workers. They would need to be converted back to the original format when passed back to the language client. I don't think this solution is practical in any way.

I'm sure there must be a simpler way of supporting symlinked directories. Perhaps we should look into how other language servers handle this.

@kanej
Copy link
Member

kanej commented May 31, 2023

Thanks @antico5, I'll take it from here 👋

@tpae
Copy link

tpae commented Sep 25, 2023

Any updates on this?

@peersky
Copy link

peersky commented Jun 13, 2024

Any update on this?

@kanej
Copy link
Member

kanej commented Jun 14, 2024

No updates, we haven't progressed the investigation.

@kanej kanej moved this from In Progress to Todo in hardhat-vscode Aug 1, 2024
@kanej kanej moved this from Todo to In Progress in hardhat-vscode Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs-more-info type:bug Something isn't working
Projects
Status: In Progress
Development

No branches or pull requests

7 participants