Fix type errors
Initial checklist
- [x] I read the support docs
- [x] I read the contributing guide
- [x] I agree to follow the code of conduct
- [x] I searched issues and couldn’t find anything (or linked relevant results below)
- [x] If applicable, I’ve added docs and tests
Description of changes
Since typical usage of unified-lint-rule involves inferring the type based on a function call, the inferred type should be public. If private types are used, TypeScript will generate types for dependant packages that use relative paths.
To solve this, public types in unified-lint-rule were moved from lib/index.js to index.js. lintRule only uses these public types.
The severity can be set as a boolean. To support this, the Label type now accepts a boolean type.
Two tests explicitly test bad input. These are annotated with a @ts-expect-error comment.
Since typical usage of unified-lint-rule involves inferring the type based on a function call, the inferred type should be public. If private types are used, TypeScript will generate types for dependant packages that use relative paths.
Why not make more types public? That would be a smaller PR, right?
The severity can be set as a boolean. To support this, the Label type now accepts a boolean type.
Can you do this in a separate PR? That way, we can focus on how to solve completely broken types here.
Also, you can use import('package-name') instead of import('../index.js'), with export maps.
I prefer that, as it shows that something is from the public API instead of from a private internal lib file.
Why not make more types public? That would be a smaller PR, right?
Which type? SeverityTuple is only used internally.
Can you do this in a separate PR? That way, we can focus on how to solve completely broken types here.
That would cause a type error in the tests.
Also, you can use
import('package-name')instead ofimport('../index.js'), with export maps. I prefer that, as it shows that something is from the public API instead of from a private internal lib file.
Nice, I like it.
Note that TypeScript 5.5’s @import annotation will allow us to add the type imports on top instead of using import('module').Type type annotations.
@remcohaszing @wooorm friendly ping 🔔
IMO this is good to go, although we might as well wait until TypeScript 5.5 now and use @import tags.
Which type? SeverityTuple is only used internally.
You describe this in your OP: “If private types are used, TypeScript will generate types for dependant packages that use relative paths.” The word private doesn’t seem right to me: if these types end up in the public API, they are thus publidc? If they are public, we should be expose to expose them too?
I feel weird about this PR because it does different things, one part of which is it uses a style that we don‘t use anywhere in unified (import(xx).yy expressions everywhere).
The other part of what this PR does (defining types in the root of a package) could perhaps be done and/or introduced everywhere, but I still wonder about my above point. And also if we’re going to do something along those lines, as I expression in the other issue, I am leaning more to using .d.ts files for type-only things.
The following code does not re-export PrivateOptions as PublicOptions:
// index.js
/**
* @typedef {import('./lib/index.js').PrivateOptions} PublicOptions
*/
It defines a new type named PublicOptions that is equivalent to PrivateOptions. It’s kind of like this:
// index.js
import { privateFunction } from './lib/index.js'
export function publicFunction(...args) {
return privateFunction(...args)
}
publicFunction and privateFunction are the equivalent, but they’re not the same function.
There are no type imports, type exports, nor type re-exports with types in JSDoc. Type imports are coming in TypeScript 5.5 though.
Calling const rule = lintRule() inferred a return type that uses Label, Option, and Severity that are defined in lib/index.js, a file that is not available through export maps. Hence those types are not part of the public API. The only way for TypeScript to access these types, is by using a relative path.
I want to provide a PoC for https://github.com/orgs/unifiedjs/discussions/238 once TypeScript 5.5 has been released, as that fixes some of the type issues we keep running into.