Repository navigation
Conversation
The workaround of 2020 for postcss#232 spells `Selector` as a type alias over an interface parameterized by itself, so that the base type of `Selector` is not `Selector` itself. The TypeScript 7.1 nightlies refuse to check that, with `TS5115` on every use of the type, reported where the type is used rather than in the declaration, so `skipLibCheck` passes over it and it reaches whoever instantiates `Selector`. Writing the self-reference back into the interface is `TS2310` again, which is what postcss#232 was about, and every other spelling costs the same on some compiler: a `Selector` that says its children are the nodes that are not selectors has to name the node union in its own base type, and that union holds `Selector`. The exclusion is given up instead, so a selector's children are typed as any node. The parser never puts a selector inside a selector, and the type test narrows by `type` before reading, so nothing in this package relies on the exclusion — but it is a loosening, and a different trade may suit this package better than this one does.
|
Thanks for the thorough table in #348, it made this easy to reason about. I would like to avoid widening the public type if we can. I built the three spellings against a consumer that narrows
So dropping the exclusion does break consumers that switch exhaustively over a selector's children, which is a realistic pattern and the reason the exclusion was there. The third spelling is the second option you listed in #348, declaring interface Selector extends Container<string, Node> {
type: "selector";
+ nodes: Array<Diff<Node, Selector>>;
}keeping |
|
Thanks for suggesting this alternative. I tested both versions of the separate
For each version, I used a consumer that narrows So declaring |
|
Thanks for testing both spellings. I get the opposite result, so one of our two consumers must differ, and rather than argue about it here is mine in full. If it fails for you, the difference will be obvious in a minute, and I will take this PR as it stands. What I measured, same consumer in all three cases, with only the declaration and the consumer in the compilation:
The exact edit for the third row, applied to the published - interface _Selector<S> extends Container<string, Diff<Node, S>> {
- type: "selector";
- }
- type Selector = _Selector<Selector>;
+ interface Selector extends Container<string, Node> {
+ type: "selector";
+ nodes: Array<Diff<Node, Selector>>;
+ }
import parser = require("./postcss-selector-parser");
function assertNever(x: never): never { throw new Error("unexpected " + x); }
export function readChildren(sel: parser.Selector): string[] {
const out: string[] = [];
for (const n of sel.nodes) {
switch (n.type) {
case "attribute": out.push(n.attribute); break;
case "class": out.push(n.value); break;
case "combinator": out.push(n.value); break;
case "comment": out.push(n.value); break;
case "id": out.push(n.value); break;
case "nesting": out.push(n.value); break;
case "pseudo": out.push(n.value); break;
case "root": out.push("root"); break;
case "string": out.push(n.value); break;
case "tag": out.push(n.value); break;
case "universal": out.push(n.value); break;
default: return assertNever(n);
}
}
return out;
}
{
"compilerOptions": {
"strict": true,
"noEmit": true,
"module": "commonjs",
"target": "es2018",
"skipLibCheck": false
},
"files": ["postcss-selector-parser.d.ts", "consumer.ts"]
}One thing to watch if you reproduce it: leaving My matrix stops at 7.0.2 because the environment I test in cannot reach packages published in the last week, so the 7.1 nightly, which is the whole point of #348, remains yours to check. If the third spelling reports TS5115 there, that settles it in favour of this PR. Separately, and this may lift the constraint you have been honouring: this repo declares no minimum TypeScript version at all. No |
Keep selector children typed as every node except selectors without making the recursive node union part of Container’s base type. This preserves exhaustive narrowing while avoiding the TypeScript 7.1 recursion error.
|
I apologize, but it seems my previous answer was incorrect. I’m not sure where, but I must have made a mistake somewhere (I guess I should just sleep at night instead of experimenting) and that’s why I was getting errors. I’ve just rechecked your version—it turns out to work perfectly on all versions of TypeScript, including tsrs. I’ve made the correction in this PR. |
Closes #348.
Selectorno longer checks on the TypeScript 7.1 nightlies, since the nightly of 2026-09-29. The workaround of #232 spells it as a type alias over an interface parameterized by itself, and the nightlies refuse to check the cycle that leaves behind, withTS5115on every use of the type.No other spelling of the exclusion survives every compiler — they are all in the issue #348 — because a
Selectorthat says its children are the nodes that are not selectors has to name the node union in its own base type, and that union holdsSelector. This PR gives the exclusion up, which is the one spelling that costs nothing anywhere:Measured with
tsc --noEmit --strict, on a consumer that narrows a selector's children bytypeand reads them:What is lost:
selector.nodeswasArray<Diff<Node, Selector>>, so narrowing a selector's children bytypecould not reach a selector. The parser never builds one,postcss-selector-parser.test.tsnarrows bytypebefore reading anything, andnpm run typecheckandnpm run testspass. But this is a loosening of the type rather than a repair of it, and if a spelling that keeps the exclusion is preferred, this is the change to argue with.Diffwas only used there, so it goes with it.