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

hardhat.config.js paths object should allow for array fields #776

Open
wighawag opened this issue Sep 6, 2020 · 18 comments
Open

hardhat.config.js paths object should allow for array fields #776

wighawag opened this issue Sep 6, 2020 · 18 comments
Labels
blocked-reason:breaking-change This is a breaking change that can only be done in a major release status:blocked Blocked by other issues or external reasons type:feature Feature request

Comments

@wighawag
Copy link
Contributor

wighawag commented Sep 6, 2020

For my plugin, I would like path config to be arrays.

Unfortunately, while buidler allow the types to be arrays, it fails at runtime because builder attempt to read them as string

@wighawag
Copy link
Contributor Author

wighawag commented Sep 6, 2020

Actually, ideally, if they could be any type, it would be even better.

@alcuadrado
Copy link
Member

Can you tell us more about what you are trying to do?

@wighawag
Copy link
Contributor Author

wighawag commented Sep 6, 2020

I basically want user to set multiple path for artifact and deployments, see : wighawag/hardhat-deploy#34

So instead of having the following in buidler.config.js

paths: {
    imports: ["node_modules/@cartesi/arbitration/build/contracts"],
    externalDeployments: {
      rinkeby: ["node_modules/@cartesi/arbitration/build/contracts"],
    }
}

I need to add the paths to a different field:

external: {
    artifacts: ["node_modules/@cartesi/arbitration/build/contracts"],
    deployments: {
      rinkeby: ["node_modules/@cartesi/arbitration/build/contracts"],
    },
  },

Actually thinking about it the deployments object could be set on the network config, like that

networks: {
    extraDeployments: ["node_modules/@cartesi/arbitration/build/contracts"]
},

But this is just an example. Basically there are use case where paths need to be arrays and potentially objects.
buidler type system allows for it, but this fails at runtime.

@fvictorio
Copy link
Member

Can you share the stack trace where it fails at runtime? If I'm understanding correctly, this shouldn't happen (I mean, fields other than the "predefined" ones shouldn't be used in any way.)

@wighawag
Copy link
Contributor Author

wighawag commented Sep 8, 2020

i forgot what was the exact error, I ll see if I can get it again, but looking at the code here : https://github.com/nomiclabs/buidler/blob/0183504555c2aed2e3ea7e63b6ddeb656ce7d4f7/packages/buidler-core/src/internal/core/config/config-resolution.ts#L95
it expected the paths to be string

@fvictorio
Copy link
Member

Oh, I see. I'm not sure what the right solution is here. I guess we could stop resolving the other entries from paths and just pass them down as they are in the buidler config, but this is a breaking change.

@mdcoon
Copy link

mdcoon commented Oct 5, 2020

Another scenario for this is for monorepos where I have contracts spread out across individual packages. I want to leverage waffle test config setting "compilerAllowedPaths" but it doesn't look like it's supported. I basically want something like this:

paths: {
compilerAllowedPaths: ['../common', '../token']
}

or paths: {
sourcesPath: ['../common/contracts', '../token/contracts'],
artifacts: ['../common/artifacts', '../token/artifacts']
}

@krasi-georgiev
Copy link

Just to add my use case as well. I want to keep all test dependency contracts in a separate folder so this is required to be able to use deploy plugin with a custom folder only for given contracts.

@fvictorio fvictorio changed the title buidle.config.js paths object should allow for array fields hardhat.config.js paths object should allow for array fields May 10, 2021
@bricedenice59
Copy link

More than one year later, is there any news about this?
I have multiple contracts scattered into multiple solution folders, each contract is a proper hardhat project. In my "main project", I have multiple tests and some depends on other contracts that are not in my main solution contract folder.
I want to deploy the other contracts by using the ethers.getContractFactory(), unfortunately it crashes.
How to do that if we cannot specify multiple path sources?
Error:
Invalid value ["./contracts","../../anotherContractPath/contracts"] for HardhatConfig.paths.sources - Expected a value of type string | undefined.
Thanks.

bricedenice59 added a commit to bricedenice59/marketplace-contract-solidity that referenced this issue Sep 1, 2022
…ve changed (multi sig wallet owners).

I had no choice to copy the multisig contract to this solution as there is no way to setup hardhat to have multiple sources from different project solutions/folders. This is an ongoing issue: NomicFoundation/hardhat#776
@fvictorio fvictorio added status:blocked Blocked by other issues or external reasons blocked-reason:breaking-change This is a breaking change that can only be done in a major release labels Dec 30, 2022
@seaona
Copy link

seaona commented Jan 24, 2023

I'm in the same situation as @bricedenice59. Is there a workaround for this issue while it's not resolved? I would really appreciate some advice here. Thank you!!

@stephancill
Copy link

Would also really appreciate this feature!

@fergarrui
Copy link

Any news on this issue?

@ghost
Copy link

ghost commented Jun 5, 2023

Any news on this? I would really need this feature, as I have an hardhat repo with some contracts, but I also have another hardhat repo with other contracts that is a submodule of the 1st one. I would like to deploy the contracts all together but I cannot because I can only set 1 single path for each kind (1 artifacts, 1 sources etc)

@lekevicius
Copy link

lekevicius commented Aug 8, 2023

Joining in. We're approaching 3 year anniversary for this issue (:

Also, I don't see how this is a breaking change: old behavior can easily be supported. Allow strings or arrays.

@fvictorio
Copy link
Member

@lekevicius I know it's hard to see how this is a breaking change, but assume you are a plugin author that uses the paths.sources somehow. The type of this is string, so your code does something assuming it is a string.

Then Hardhat updates to a new patch/minor version and suddenly sources is string | string[]. Your code very likely won't work when you receive an array. So it's not a breaking change from a user perspective, but it can be from a plugin author perspective.

That being said:

  • We are not against introducing minor breaking changes within the same major version; we've done it in the past if we think the risk is low enough. In this case my instinct is that it's not low enough, but I could be wrong.
  • This is easy to do with a plugin or overriding a subtask. I know that that approach has way more friction than having this as a built-in feature, but the fact that there is a workaround also makes me err on the side of not introducing a breaking change.

Hopefully that makes sense, happy to answer any questions about this.

@tariqkb
Copy link

tariqkb commented Sep 6, 2023

This is easy to do with a plugin or overriding a subtask. I know that that approach has way more friction than having this as a built-in feature, but the fact that there is a workaround also makes me err on the side of not introducing a breaking change.

Hello @fvictorio, do you have an example I can look at to do this? Is it TASK_COMPILE_SOLIDITY_GET_SOURCE_PATHS that needs overriding?

@fvictorio
Copy link
Member

Something like this should work:

const glob = require("glob");
const { TASK_COMPILE_SOLIDITY_GET_SOURCE_PATHS } = require("hardhat/builtin-tasks/task-names");
const path = require("path");

subtask(TASK_COMPILE_SOLIDITY_GET_SOURCE_PATHS).setAction(async (_, hre, runSuper) => {
  const paths = await runSuper();

  const otherDirectoryGlob = path.join(hre.config.paths.root, "more-contracts", "**", "*.sol");
  const otherPaths = glob.sync(otherDirectoryGlob);

  return [...paths, ...otherPaths];
});

@cekingx
Copy link

cekingx commented Oct 12, 2023

Something like this should work:

const glob = require("glob");
const { TASK_COMPILE_SOLIDITY_GET_SOURCE_PATHS } = require("hardhat/builtin-tasks/task-names");
const path = require("path");

subtask(TASK_COMPILE_SOLIDITY_GET_SOURCE_PATHS).setAction(async (_, hre, runSuper) => {
  const paths = await runSuper();

  const otherDirectoryGlob = path.join(hre.config.paths.root, "more-contracts", "**", "*.sol");
  const otherPaths = glob.sync(otherDirectoryGlob);

  return [...paths, ...otherPaths];
});

thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked-reason:breaking-change This is a breaking change that can only be done in a major release status:blocked Blocked by other issues or external reasons type:feature Feature request
Projects
Status: Blocked
Development

No branches or pull requests