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

fix(format): allow calls with partial arguments #198

Merged
merged 1 commit into from
Jan 9, 2025
Merged

fix(format): allow calls with partial arguments #198

merged 1 commit into from
Jan 9, 2025

Conversation

43081j
Copy link
Contributor

@43081j 43081j commented Jan 9, 2025

In node, the following are valid calls of format:

format({ file: 'foo' });
// node: foo
// pathe: fooundefined

format({ dir: '/foo', name: 'bar' });
// node: /foo/bar
// pathe: /foo/fileundefined

format({ dir: '/foo', ext: 'bar' });
// node: /foo/.bar
// pathe: undefinedext

format({ ext: 'foo' });
// node: .foo
// pathe: undefined.ext

format({ ext: '.foo' });
// node: .foo
// pathe: undefinedext

The last one is a little nonsensical, but the other two are valid (extensionless files for example). We should also be matching node's behaviour of course.

@pi0 this was actually a PR to enable strict mode in typescript, that then uncovered the bug. Let me know if you want me to chop the PR up into two PRs as this also adds some casts to make the compiler happy

@43081j
Copy link
Contributor Author

43081j commented Jan 9, 2025

im going to split the changes into two PRs since i didn't notice the tests have many strict errors

@pi0 pi0 changed the title fix: allow format calls with one side of (file + ext) missing fix(format): allow calls with partial arguments Jan 9, 2025
In node, the following are valid calls of `format`:

```ts
format({ file: 'foo' });
// foo

format({ dir: '/foo', name: 'bar' });
// /foo/bar

format({ dir: '/foo', ext: 'bar' });
// /foo/.bar
```

The last one is a little nonsensical, but the other two are valid
(extensionless files for example). We should also be matching node's
behaviour of course.

Also enables `strict: true` in TypeScript, which is what caught this.
@43081j
Copy link
Contributor Author

43081j commented Jan 9, 2025

here you go i've dropped the typescript changes for now 👍

@pi0 pi0 merged commit 4353c6a into main Jan 9, 2025
1 check passed
@43081j 43081j deleted the strict-ts branch January 9, 2025 15:43
@pi0
Copy link
Member

pi0 commented Jan 9, 2025

@43081j are you going to work on strict type fixes? (here is unjs new tsconfig for reference to adopt)

@43081j
Copy link
Contributor Author

43081j commented Jan 9, 2025

@43081j are you going to work on strict type fixes? (here is unjs new tsconfig for reference to adopt)

ill take a look 👍

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.

2 participants