feat: add validation for readOptions with zod #826

nowyDEV · 2023-09-13T21:34:14Z

PR Checklist

Addresses an existing open issue: fixes 🚀 Feature: Validate parseArgs values with Zod #784
That issue was marked as status: accepting prs
Steps in CONTRIBUTING.md were taken

Overview

Removed type assertions and replaced them with zod validation.

src/shared/options/readOptions.ts

codecov · 2023-09-13T21:39:23Z

Codecov Report

Merging #826 (bcb68aa) into main (c66fdda) will increase coverage by 0.56%.
Report is 2 commits behind head on main.
The diff coverage is 80.50%.

@@            Coverage Diff             @@
##             main     #826      +/-   ##
==========================================
+ Coverage   91.61%   92.18%   +0.56%     
==========================================
  Files          84       84              
  Lines        4161     4220      +59     
  Branches      248      261      +13     
==========================================
+ Hits         3812     3890      +78     
+ Misses        349      330      -19

Flag	Coverage Δ
create	`71.95% <78.12%> (-0.09%)`	⬇️
initialize	`34.61% <81.25%> (+0.52%)`	⬆️
migrate	`73.27% <79.16%> (-0.08%)`	⬇️
unit	`52.51% <78.81%> (+4.81%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/shared/types.ts	`0.00% <0.00%> (ø)`
src/shared/options/augmentOptionsWithExcludes.ts	`62.09% <50.00%> (-0.49%)`	⬇️
src/bin/index.ts	`100.00% <100.00%> (ø)`
src/bin/mode.ts	`100.00% <100.00%> (ø)`
src/create/index.ts	`89.04% <100.00%> (+0.15%)`	⬆️
src/initialize/index.ts	`97.77% <100.00%> (+0.05%)`	⬆️
src/migrate/index.ts	`97.56% <100.00%> (+0.06%)`	⬆️
src/shared/options/readOptions.ts	`89.23% <100.00%> (+15.89%)`	⬆️
src/steps/uninstallPackages.ts	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

src/shared/options/readOptions.ts

nowyDEV · 2023-09-14T12:31:52Z

Looks like adding zod breaks migration script, not sure how to deal with this 😿

JoshuaKGoldberg

Great stuff! Really excited about this - it's a nice and clean solution that removes a bunch of type assertions. 🔥

Left some comments, but I'm not super picky on any of them. What do you think?

JoshuaKGoldberg · 2023-09-14T19:27:56Z

pnpm-lock.yaml

@@ -7656,4 +7659,3 @@ packages:

  /[email protected]:
    resolution: {integrity: sha512-wvWkphh5WQsJbVk1tbx1l1Ly4yg+XecD+Mq280uBGt9wa5BKSWf4Mhp6GmrkPixhMxmabYY7RbzlwVP32pbGCg==}
-    dev: true


[Comment] Haha, nice to see zod already included!

src/shared/options/readOptions.test.ts

src/shared/options/readOptions.ts

JoshuaKGoldberg · 2023-09-18T15:02:39Z

src/bin/index.ts

@@ -50,7 +51,9 @@ export async function bin(args: string[]) {
 		return 1;
 	}

-	const { code, options } = await { create, initialize, migrate }[mode](args);
+	const { code, options, zodError } = await { create, initialize, migrate }[


[Style] As a total non-blocking aside, I went back and forth between making an object like const runners = { create, ... } in the root of the file or in the function. It might look cleaner here? Not a blocker whichever way you prefer 😄 just raising that I'm not picky.

I think making a separate variable might make it a little bit more readable.

JoshuaKGoldberg · 2023-09-18T15:05:17Z

src/shared/options/readOptions.test.ts

+			},
+		});
+	});
+});


[Praise] 💯 love it! No notes!

JoshuaKGoldberg

Looking great! I think the only blocking request on my end is on the prompts.outro. Otherwise everything is either praise or a nitpick. Nice! 🔥

JoshuaKGoldberg · 2023-09-18T15:08:05Z

src/bin/index.test.ts

+		mockInitialize.mockResolvedValue({
+			code: 2,
+			options: {},
+			zodError: !validationResult.success ? validationResult.error : null,


[Cleanup] Nitpicking: no need for a runtime check, we can use a type assertion:

Suggested change

zodError: !validationResult.success ? validationResult.error : null,

zodError: (validationResult as z.SafeParseError<{ email: string }>).error,

A little more code, but IMO more true to what the code is trying to do.

JoshuaKGoldberg · 2023-09-18T15:13:08Z

src/bin/index.ts

+
+		if (zodError) {
+			const validationError = fromZodError(zodError);
+			prompts.outro(chalk.red(validationError));


prompts.outro

[Bug] The terminal output looks a little buggy with two outros:

┌ Welcome to create-typescript-app ! 🎉 │ │ ⚠️ This template is early stage, opinionated, and not endorsed by the TypeScript team. ⚠️ │ ⚠️ If any tooling it sets displeases you, you can always remove that portion manually. ⚠️ │ ● Tip: to run again with the same input values, use: npx create-typescript-app --mode create --base everything --email wat │ │ └ Validation error: Invalid email at "email" └ Operation cancelled. Exiting - maybe another time? 👋

How about switching to a logLine? I think the ideal would be a blank line around the validatione rror:

┌ Welcome to create-typescript-app ! 🎉 │ │ ⚠️ This template is early stage, opinionated, and not endorsed by the TypeScript team. ⚠️ │ ⚠️ If any tooling it sets displeases you, you can always remove that portion manually. ⚠️ │ ● Tip: to run again with the same input values, use: npx create-typescript-app --mode create --base everything --email wat │ │ Validation error: Invalid email at "email" │ └ Operation cancelled. Exiting - maybe another time? 👋

JoshuaKGoldberg

Super! This is great - really happy about the reduction of assertions in the codebase. Thanks! ✨

github-actions · 2023-09-18T19:17:53Z

🎉 This is included in version v1.29.40 🎉

The release is available on:

Cheers! 📦🚀

nowyDEV added 3 commits September 13, 2023 22:56

fininsh implementation

3927eb1

fix type issues

01db7a2

refactor

273d5b9

nowyDEV commented Sep 13, 2023

View reviewed changes

src/shared/options/readOptions.ts Outdated Show resolved Hide resolved

nowyDEV commented Sep 13, 2023

View reviewed changes

src/shared/options/readOptions.ts Outdated Show resolved Hide resolved

add error handling

e96f283

nowyDEV commented Sep 14, 2023

View reviewed changes

src/shared/options/readOptions.ts Outdated Show resolved Hide resolved

nowyDEV commented Sep 14, 2023

View reviewed changes

src/shared/options/readOptions.ts Outdated Show resolved Hide resolved

nowyDEV added 4 commits September 14, 2023 11:01

fix knip check

99e2c0a

refactor

8624778

add test for zod validation

f2f3a9b

fix spelling

dabba56

nowyDEV added 3 commits September 14, 2023 16:01

add test case for successful run

6e9bdc7

fix tests

8b790d4

fix tests v2

3e7c57c

JoshuaKGoldberg requested changes Sep 15, 2023

View reviewed changes

JoshuaKGoldberg added the status: waiting for author Needs an action taken by the original poster label Sep 15, 2023

nowyDEV and others added 2 commits September 16, 2023 00:40

address review comments

8ad3aa6

Merge branch 'main' into validate-options-zod

9a71569

nowyDEV requested a review from JoshuaKGoldberg September 15, 2023 22:45

github-actions bot removed the status: waiting for author Needs an action taken by the original poster label Sep 15, 2023

nowyDEV and others added 3 commits September 16, 2023 00:50

cleanup added package

926715f

Merge branch 'main' into validate-options-zod

abef694

Merge branch 'main' into validate-options-zod

78aa8a3

JoshuaKGoldberg reviewed Sep 18, 2023

View reviewed changes

src/shared/options/readOptions.test.ts

},

});

});

});

Copy link

Owner

JoshuaKGoldberg Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Praise] 💯 love it! No notes!

nowyDEV reacted with rocket emoji

JoshuaKGoldberg requested changes Sep 18, 2023

View reviewed changes

JoshuaKGoldberg added the status: waiting for author Needs an action taken by the original poster label Sep 18, 2023

nowyDEV and others added 4 commits September 18, 2023 19:56

fix error display inside prompt

b6ea877

use assertion instead of runtime check

763c1a9

extract runners to separate variable

1334977

Merge branch 'main' into validate-options-zod

cd18fe1

nowyDEV requested a review from JoshuaKGoldberg September 18, 2023 18:01

github-actions bot removed the status: waiting for author Needs an action taken by the original poster label Sep 18, 2023

fix test

bcb68aa

JoshuaKGoldberg approved these changes Sep 18, 2023

View reviewed changes

JoshuaKGoldberg merged commit f4e5830 into JoshuaKGoldberg:main Sep 18, 2023
15 of 16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add validation for readOptions with zod #826

feat: add validation for readOptions with zod #826

nowyDEV commented Sep 13, 2023

codecov bot commented Sep 13, 2023 •

edited

Loading

nowyDEV commented Sep 14, 2023

JoshuaKGoldberg left a comment

JoshuaKGoldberg Sep 14, 2023 •

edited

Loading

JoshuaKGoldberg Sep 18, 2023

nowyDEV Sep 18, 2023

JoshuaKGoldberg Sep 18, 2023

JoshuaKGoldberg left a comment

JoshuaKGoldberg Sep 18, 2023

JoshuaKGoldberg Sep 18, 2023

JoshuaKGoldberg left a comment

github-actions bot commented Sep 18, 2023

	zodError: !validationResult.success ? validationResult.error : null,
	zodError: (validationResult as z.SafeParseError<{ email: string }>).error,

feat: add validation for readOptions with zod #826

feat: add validation for readOptions with zod #826

Conversation

nowyDEV commented Sep 13, 2023

PR Checklist

Overview

codecov bot commented Sep 13, 2023 • edited Loading

Codecov Report

nowyDEV commented Sep 14, 2023

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg Sep 18, 2023

Choose a reason for hiding this comment

nowyDEV Sep 18, 2023

Choose a reason for hiding this comment

JoshuaKGoldberg Sep 18, 2023

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Sep 18, 2023

Choose a reason for hiding this comment

JoshuaKGoldberg Sep 18, 2023

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 18, 2023

codecov bot commented Sep 13, 2023 •

edited

Loading

JoshuaKGoldberg Sep 14, 2023 •

edited

Loading