resolveGlobalArguments implementation & testing #5414

schaable · 2024-06-19T22:00:21Z

Part of #5379

This should be easier to review by reviewing each commit individually.

Merge this after #5399

vercel · 2024-06-19T22:00:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hardhat	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 25, 2024 2:27pm

changeset-bot · 2024-06-19T22:00:25Z

⚠️ No Changeset found

Latest commit: 665a755

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

v-next/core/src/types/global-parameters.ts

zoeyTM · 2024-06-24T04:48:34Z

@schaable now that #5417 has been merged, this PR will need rebasing and will need to also follow the new name scheme. i.e. resolveGlobalArguments becomes resolveGlobalOptions, etc.

v-next/hardhat/src/internal/cli/main.ts

schaable · 2024-06-24T13:43:15Z

@schaable now that #5417 has been merged, this PR will need rebasing and will need to also follow the new name scheme. i.e. resolveGlobalArguments becomes resolveGlobalOptions, etc.

done!

alcuadrado · 2024-06-24T19:18:53Z

v-next/core/src/internal/global-options.ts

+
+    let parsedValue: ParameterValue;
+    if (value !== undefined) {
+      parsedValue = parseParameterValue(value, option.parameterType, name);


I believe this is trying to parse the userProvidedGlobalOptions's values.

if those are provided, they should already be parsed

Fixed in 665a755

alcuadrado · 2024-06-24T19:20:16Z

v-next/core/src/types/global-options.ts

@@ -25,7 +29,7 @@ export interface GlobalOption<T extends ParameterType = ParameterType> {
 * Runtime Environment.
 */
 // eslint-disable-next-line @typescript-eslint/no-empty-interface -- To be used through module augmentation
-export interface GlobalOptions {}
+export interface GlobalOptions extends Record<string, ParameterValue> {}


I think this will be a bit annoying, as you can do globalOptions.thisDoesntExist and you wouldn't get a type error

What motivated it? We may find an alternative

If I remember correctly, this was done to avoid an error when assigning the parsed value to the global options. As the interface is empty, this throws a TS error: globalOptions[name] = parsedValue;

Element implicitly has an 'any' type because expression of type 'string' can't be used to index type 'GlobalOptions'.
No index signature with a parameter of type 'string' was found on type 'GlobalOptions'.ts(7053)

Fixed in 22abbd5

alcuadrado · 2024-06-24T19:22:52Z

v-next/core/test/utils.ts

@@ -0,0 +1,30 @@
+import { afterEach } from "node:test";
+
+export function createTestEnvManager() {


this is pretty nice

alcuadrado · 2024-06-24T19:23:47Z

v-next/core/test/internal/global-options.ts

+
+      const globalOptions = resolveGlobalOptions(
+        {
+          param1: "false",


this is an example of a param that should already be parsed

I think a good way to think about this is: first the cli parses the cli arguments, then the hre is initialized, using those arguments, which requires resolving like this.

alcuadrado · 2024-06-24T19:26:17Z

v-next/core/src/internal/parameters.ts

+/**
+ * Parses a parameter value from a string to the corresponding type.
+ */
+// TODO: this code is duplicated in v-next/hardhat/src/internal/cli/main.ts


I think exposing these things from core makes sense. We already expose functions like this. e.g. resolvePluginList.

Agreed. I'll do it on a separate PR.

Issue created: #5431

alcuadrado

Left a few small-ish comments

schaable added the v-next A Hardhat v3 development task label Jun 19, 2024

schaable self-assigned this Jun 19, 2024

github-actions bot added the status:ready This issue is ready to be worked on label Jun 19, 2024

schaable commented Jun 19, 2024

View reviewed changes

v-next/core/src/types/global-parameters.ts Outdated Show resolved Hide resolved

Base automatically changed from build-global-param to v-next June 19, 2024 22:25

schaable force-pushed the resolve-global-args branch from 5fcc4ea to 3b674d1 Compare June 19, 2024 22:40

vercel bot deployed to Preview June 19, 2024 22:41 View deployment

schaable added 5 commits June 24, 2024 09:59

Add camelToSnakeCase

9bc3a6c

Add parameter parsing

0f5453a

Improve types

5fc1ba3

Add global args resolution

02d6b97

Add tests to resolveGlobalArguments

d33a223

schaable force-pushed the resolve-global-args branch from 3b674d1 to d33a223 Compare June 24, 2024 13:25

vercel bot deployed to Preview June 24, 2024 13:26 View deployment

Fix build errors

0eca4fa

vercel bot deployed to Preview June 24, 2024 13:39 View deployment

schaable commented Jun 24, 2024

View reviewed changes

v-next/hardhat/src/internal/cli/main.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Jun 24, 2024

View reviewed changes

schaable added 2 commits June 25, 2024 11:25

Fix type error

22abbd5

Parse only env vars

665a755

vercel bot deployed to Preview June 25, 2024 14:27 View deployment

schaable requested a review from alcuadrado June 25, 2024 14:30

alcuadrado approved these changes Jun 25, 2024

View reviewed changes

schaable merged commit b62a981 into v-next Jun 25, 2024
50 checks passed

schaable deleted the resolve-global-args branch June 25, 2024 15:20

github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolveGlobalArguments implementation & testing #5414

resolveGlobalArguments implementation & testing #5414

schaable commented Jun 19, 2024

vercel bot commented Jun 19, 2024 •

edited

Loading

changeset-bot bot commented Jun 19, 2024 •

edited

Loading

zoeyTM commented Jun 24, 2024

schaable commented Jun 24, 2024

alcuadrado Jun 24, 2024

alcuadrado Jun 24, 2024

schaable Jun 25, 2024

alcuadrado Jun 24, 2024

alcuadrado Jun 24, 2024

schaable Jun 24, 2024

schaable Jun 25, 2024

alcuadrado Jun 24, 2024

alcuadrado Jun 24, 2024

alcuadrado Jun 24, 2024

alcuadrado Jun 24, 2024

schaable Jun 24, 2024

schaable Jun 25, 2024

alcuadrado left a comment

		@@ -0,0 +1,30 @@
		import { afterEach } from "node:test";

		export function createTestEnvManager() {

resolveGlobalArguments implementation & testing #5414

resolveGlobalArguments implementation & testing #5414

Conversation

schaable commented Jun 19, 2024

vercel bot commented Jun 19, 2024 • edited Loading

changeset-bot bot commented Jun 19, 2024 • edited Loading

⚠️ No Changeset found

zoeyTM commented Jun 24, 2024

schaable commented Jun 24, 2024

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

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

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

Choose a reason for hiding this comment

alcuadrado left a comment

Choose a reason for hiding this comment

vercel bot commented Jun 19, 2024 •

edited

Loading

changeset-bot bot commented Jun 19, 2024 •

edited

Loading