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

Rewrite in TypeScript #252

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

tommy-mitchell
Copy link
Contributor

@tommy-mitchell tommy-mitchell commented Jan 28, 2024

Per #233.

@tommy-mitchell tommy-mitchell changed the title rename Rewrite in TypeScript Jan 28, 2024
@tommy-mitchell tommy-mitchell mentioned this pull request Jan 28, 2024
9 tasks
readme.md Outdated Show resolved Hide resolved
source/index.ts Outdated Show resolved Hide resolved
source/index.ts Outdated Show resolved Hide resolved
source/options.ts Outdated Show resolved Hide resolved
test/test.ts Outdated
Comment on lines 23 to 25
'--': true as unknown as {type: 'string'; isMultiple: true},
// TODO: can not model due to microsoft/TypeScript#17867
// A potential workaround is to use a symbol for '--' -> {flags: [argvDashDash]: true}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the "rest index type" issues

Copy link
Owner

Choose a reason for hiding this comment

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

Use a proper URL over microsoft/TypeScript#17867

test/test.ts Outdated Show resolved Hide resolved
@tommy-mitchell
Copy link
Contributor Author

May have missed some comments, but that's the bulk of where I'm needing clarification. The other TODOs I have are reminders to fix type issues.

@sindresorhus
Copy link
Owner

FYI: e9a55cd

@tommy-mitchell
Copy link
Contributor Author

Do we support a shorthand for setting flags?

const cli = meow({
	importMeta: import.meta
	flags: {
		rainbow: 'boolean',
		unicorn: 'string',
	},
});

@sindresorhus
Copy link
Owner

Do we support a shorthand for setting flags?

No. That's not something I want to support either.

@tommy-mitchell
Copy link
Contributor Author

Got it. I think this test case (and maybe another?) had confused me before and I was remembering something about it:

meow/test/errors.js

Lines 38 to 40 in c97c031

test('flag declared in kebab-case is an error', meowThrows, {
flags: {'kebab-case': 'boolean', test: 'boolean', 'another-one': 'boolean'},
}, {message: 'Flag keys may not contain \'-\'. Invalid flags: `kebab-case`, `another-one`'});

@sindresorhus
Copy link
Owner

That test case is incorrect.

@tommy-mitchell
Copy link
Contributor Author

Updated to reflect changes from other PRs. Type tests are failing due to tsd not seeing minimist-options.d.ts.

source/options.ts Outdated Show resolved Hide resolved
@tommy-mitchell
Copy link
Contributor Author

CI is failing due to sindresorhus/type-fest#824

@tommy-mitchell
Copy link
Contributor Author

I'm not sure how to fix the timeout issue, I get it sometimes locally when running npm test but never with npx ava.

@sindresorhus
Copy link
Owner

Maybe try downgrading to ava@5 for now.

@tommy-mitchell
Copy link
Contributor Author

Didn't work, guess it's something on our end. Now it doesn't even say which test timed out.

@sindresorhus
Copy link
Owner

Works for me now: 64f0915

@sindresorhus
Copy link
Owner

Hmm, or maybe not. Maybe the problem here is tsimp. I never had any problems with tsx. Maybe worth trying that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants