Skip to content

Commit 0fb6f97

Browse files
baozhoutaoclaude
andauthored
fix(lint)!: withdraw absolute-colspan-discouraged — its premise was measured false and its recommendation measured worse than the thing it warned about (#17671)
* fix(lint): withdraw absolute-colspan-discouraged — its premise was measured false and its recommendation measured worse (#17328) The rule fired on every authored `colSpan`, `colSpan: 1` included, and asserted that a fixed span "only aligns at one width". Browser measurement at all three surface widths the message names shows the renderer clamps the span to the section's column count: overflow is 0px in every configuration, including `colSpan: 4` in a 3-column section. The hint's recommended alternative, `span: 'full'`, compiles to the same class as `colSpan: 4` and renders pixel-identical to authoring nothing at the modal width. With no authored colSpan shape left that misbehaves there was nothing to re-ground, so the rule is withdrawn rather than narrowed. `colSpan` itself is untouched and still parses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU * chore(changeset): record the withdrawal of absolute-colspan-discouraged (#17328) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU --------- Co-authored-by: claude <noreply@anthropic.com>
1 parent 4c42fd1 commit 0fb6f97

6 files changed

Lines changed: 188 additions & 74 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
fix(lint)!: `absolute-colspan-discouraged` is withdrawn — its premise was measured false in a browser, and the alternative it recommended measured worse than the thing it warned about (#17328)
6+
7+
<!-- adr-0087: not-required (no-migration-prescription) nothing an author writes moves: `FormField.colSpan` is untouched in `packages/spec`, still parses, and every stored form view keeps its shape and its rendering. The only surface that moves is one exported TypeScript rule-id constant in `@objectstack/lint` plus the advisory finding it named — neither has a metadata representation, so `objectstack migrate meta` has nothing to reach and the ledger serves nobody affected. -->
8+
9+
**BREAKING** — `@objectstack/lint` no longer exports `FORM_COLSPAN_ABSOLUTE`, and
10+
`validateFormLayout` no longer emits the `absolute-colspan-discouraged` finding. A
11+
TypeScript consumer that imported that constant (to suppress the rule, or to route it)
12+
stops compiling on the import, and the compiler names the site — a more precise channel
13+
than any release note. Authored metadata is untouched: `FormField.colSpan` is unchanged
14+
and still valid.
15+
16+
The rule fired on **every** authored `colSpan`, `colSpan: 1` included, and asserted a
17+
rendering consequence: the form's column count is derived per surface (mobile 1 / modal 2
18+
/ page 3-4), so a fixed span "only aligns at one width". Measured in Chromium on a real
19+
authored 3-column section at all three of the widths that sentence names (390 / 720 /
20+
1700), that misalignment does not happen. The renderer emits one container-query-scoped
21+
span class clamped to the section's declared column count, so the cell starts at a real
22+
column boundary at every width and rendered overflow is 0px in every configuration —
23+
including `colSpan: 4` in a 3-column section, the case that would overflow if the clamp
24+
did not work. The clamp is precisely why the claim was false, and the rule's own file
25+
already recorded the clamp a few lines above the claim.
26+
27+
The hint was the sharper defect. It steered authors to `span: 'full'`, which compiles to
28+
the same class as `colSpan: 4` (`@2xl:col-span-3`) and measures byte-identically: the rule
29+
warned about one spelling and recommended the other, and they are the same thing. At the
30+
modal width `span: 'full'` renders pixel-identical to authoring nothing at all, so an
31+
author who complied was left worse off than one who ignored it.
32+
33+
With no authored `colSpan` shape left that misbehaves there was nothing to re-ground, so
34+
the rule is withdrawn rather than narrowed: `colSpan: 1` emits no class at all, a
35+
`colSpan` within the column count renders exactly as authored, and one above it clamps.
36+
Every test that pinned the rule's wording or its firing set was re-judged in place with
37+
the reason recorded, never deleted, and each re-judged pin is paired with a live finding
38+
on the same fixture so that a walk which stopped reaching the site could not pass as a
39+
withdrawal.

‎packages/lint/src/authoring-rule-wiring.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -596,10 +596,14 @@ describe('authoring-rule registry wiring (#4409)', () => {
596596
},
597597
{},
598598
);
599-
expect(findings.map((f) => f.rule).sort()).toEqual([
600-
'absolute-colspan-discouraged',
601-
'form-field-unknown',
602-
]);
599+
// [#17328] RE-JUDGED, not deleted: the fixture used to earn a second rule
600+
// id, `absolute-colspan-discouraged`, which was withdrawn after browser
601+
// measurement falsified the misalignment it asserted. The `colSpan: 2` is
602+
// left on the fixture on purpose — an entry-object field is the shape this
603+
// adapter has to survive — and `form-field-unknown` still carries the whole
604+
// point of the test: the registry entry's `run` really returns the finding
605+
// a broken stack earns, on all three commands.
606+
expect(findings.map((f) => f.rule).sort()).toEqual(['form-field-unknown']);
603607
});
604608

605609
it('every ledger entry is still an exported rule', () => {

‎packages/lint/src/authoring-rules.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,10 +1026,12 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
10261026
run: (stack) => validateSemanticRoles(stack),
10271027
},
10281028
// #2578 / #4449 — a form section's field reference that resolves to nothing
1029-
// (silently not rendered) and an absolute `colSpan` under a per-surface
1030-
// derived column count. Advisory: the renderer skips the unknown field and
1031-
// clamps the span, so nothing is broken — but each is almost certainly an
1032-
// authoring mistake, and until #4449 this rule ran on no command at all.
1029+
// (silently not rendered), and a `section.group` naming a field group the
1030+
// object does not declare. Advisory: the renderer skips the unknown field, so
1031+
// nothing is broken — but each is almost certainly an authoring mistake, and
1032+
// until #4449 this rule ran on no command at all. [#17328] It used to carry a
1033+
// third finding, `absolute-colspan-discouraged`; that one was withdrawn —
1034+
// `validate-form-layout.ts`'s module note carries the measurement.
10331035
// Pure structured-metadata walk (no lazy dependency), so wiring it to all
10341036
// three costs nothing measurable.
10351037
{

‎packages/lint/src/index.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,17 @@ export {
227227
} from './validate-semantic-roles.js';
228228
export type { SemanticRoleFinding, SemanticRoleSeverity } from './validate-semantic-roles.js';
229229

230+
// [#17328] `FORM_COLSPAN_ABSOLUTE` / `absolute-colspan-discouraged` was
231+
// WITHDRAWN, not renamed: browser measurement at all three surface widths
232+
// falsified the misalignment it asserted, and its recommended alternative
233+
// measured worse than the thing it warned about. The argument is in
234+
// `validate-form-layout.ts`'s module note. Removing the constant rather than
235+
// leaving it exported is deliberate: a rule id on the public surface reads as a
236+
// check the platform performs (Prime Directive #10), and the compiler is the
237+
// precise channel for telling a consumer that suppresses it.
230238
export {
231239
validateFormLayout,
232240
FORM_FIELD_UNKNOWN,
233-
FORM_COLSPAN_ABSOLUTE,
234241
FORM_SECTION_GROUP_UNKNOWN,
235242
} from './validate-form-layout.js';
236243
export type { FormLayoutFinding, FormLayoutSeverity } from './validate-form-layout.js';

‎packages/lint/src/validate-form-layout.test.ts‎

Lines changed: 88 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { defineStack, normalizeStackInput } from '@objectstack/spec';
55
import {
66
validateFormLayout,
77
FORM_FIELD_UNKNOWN,
8-
FORM_COLSPAN_ABSOLUTE,
98
FORM_SECTION_GROUP_UNKNOWN,
109
} from './validate-form-layout.js';
1110

@@ -38,7 +37,7 @@ const groupedObjects = [
3837
];
3938

4039
describe('validateFormLayout (#2578)', () => {
41-
it('is clean for a well-formed multi-column form (known fields, no colSpan)', () => {
40+
it('is clean for a well-formed multi-column form (known fields)', () => {
4241
const stack = {
4342
objects,
4443
views: [
@@ -74,7 +73,19 @@ describe('validateFormLayout (#2578)', () => {
7473
expect(findings[0].path).toBe('views[0].sections[0].fields[1]');
7574
});
7675

77-
it('discourages absolute colSpan and steers to span', () => {
76+
// [#17328] RE-JUDGED, not deleted. This test used to be the pin on the
77+
// `absolute-colspan-discouraged` wording — it asserted the finding fired on
78+
// `colSpan: 2` and that the hint said `span: 'full'`. Both halves were
79+
// measured wrong in Chromium at the three surface widths the message named:
80+
// the renderer clamps the span to the section's column count, so an absolute
81+
// `colSpan` is grid-aligned at EVERY width (0px overflow in every
82+
// configuration), and `span: 'full'` — the recommended alternative — compiles
83+
// to the same class as `colSpan: 4` and renders pixel-identical to authoring
84+
// nothing at the modal width. The rule was withdrawn. What the pin asserts
85+
// now is that withdrawal, and it is deliberately PAIRED with a live finding
86+
// on the same section: a bare `expect([])` here would keep passing if the
87+
// walk stopped reaching this site at all.
88+
it('[#17328] an absolute colSpan is no longer a finding — the rule was withdrawn', () => {
7889
const stack = {
7990
objects,
8091
views: [
@@ -85,42 +96,70 @@ describe('validateFormLayout (#2578)', () => {
8596
},
8697
],
8798
};
88-
const findings = validateFormLayout(stack);
89-
expect(findings).toHaveLength(1);
90-
expect(findings[0].rule).toBe(FORM_COLSPAN_ABSOLUTE);
91-
expect(findings[0].hint).toContain("span: 'full'");
92-
expect(findings[0].path).toBe('views[0].sections[0].fields[1].colSpan');
93-
});
99+
expect(validateFormLayout(stack)).toEqual([]);
94100

95-
it('reports both rules for the same field independently', () => {
96-
const stack = {
101+
// POSITIVE CONTROL on the identical shape: the walk does reach
102+
// `sections[0].fields[1]`, so the empty result above is a verdict about
103+
// `colSpan` and not about an unread site.
104+
const reached = validateFormLayout({
97105
objects,
98106
views: [
99107
{
100108
name: 'contract_form',
101109
data: { provider: 'object', object: 'contract' },
102-
sections: [{ columns: 2, fields: [{ field: 'ghost', colSpan: 3 }] }],
110+
sections: [{ columns: 2, fields: ['name', { field: 'ghost_amount', colSpan: 2 }] }],
103111
},
104112
],
105-
};
106-
const rules = validateFormLayout(stack).map(f => f.rule).sort();
107-
expect(rules).toEqual([FORM_COLSPAN_ABSOLUTE, FORM_FIELD_UNKNOWN].sort());
113+
});
114+
expect(reached.map((f) => `${f.rule}@${f.path}`)).toEqual([
115+
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[1]`,
116+
]);
108117
});
109118

110-
it('skips reference-checking when the bound object cannot be resolved', () => {
119+
// [#17328] RE-JUDGED, not deleted. The same fixture used to earn TWO findings
120+
// (`form-field-unknown` + `absolute-colspan-discouraged`). With the colSpan
121+
// rule withdrawn the surviving assertion is the one that still carries
122+
// information: a `colSpan` on an entry does not suppress, duplicate or
123+
// relocate the reference finding the entry independently earns.
124+
it('a colSpan on the entry does not disturb the reference finding it earns', () => {
111125
const stack = {
112126
objects,
113127
views: [
114128
{
115-
name: 'orphan_form',
116-
data: { provider: 'object', object: 'does_not_exist' },
117-
sections: [{ columns: 2, fields: ['whatever', { field: 'x', colSpan: 2 }] }],
129+
name: 'contract_form',
130+
data: { provider: 'object', object: 'contract' },
131+
sections: [{ columns: 2, fields: [{ field: 'ghost', colSpan: 3 }] }],
118132
},
119133
],
120134
};
121-
const findings = validateFormLayout(stack);
122-
// No form-field-unknown (object unresolved), but colSpan is still flagged.
123-
expect(findings.map(f => f.rule)).toEqual([FORM_COLSPAN_ABSOLUTE]);
135+
expect(validateFormLayout(stack).map((f) => `${f.rule}@${f.path}`)).toEqual([
136+
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[0]`,
137+
]);
138+
});
139+
140+
// [#17328] RE-JUDGED, not deleted. The old assertion was
141+
// `[FORM_COLSPAN_ABSOLUTE]` — the colSpan rule was the only thing that could
142+
// still speak on an unresolvable binding, which is what made this test
143+
// non-empty. With that rule withdrawn the verdict is `[]`, so the test is
144+
// PAIRED with the same stack under a resolvable binding; otherwise a walk
145+
// that stopped visiting orphan views entirely would look identical.
146+
it('skips reference-checking when the bound object cannot be resolved', () => {
147+
const sections = [{ columns: 2, fields: ['whatever', { field: 'x', colSpan: 2 }] }];
148+
expect(validateFormLayout({
149+
objects,
150+
views: [{ name: 'orphan_form', data: { provider: 'object', object: 'does_not_exist' }, sections }],
151+
})).toEqual([]);
152+
153+
// POSITIVE CONTROL: the identical sections, bound to an object that DOES
154+
// resolve, report both dangling names — so the silence above is the
155+
// unresolved binding, not an unvisited site.
156+
expect(validateFormLayout({
157+
objects,
158+
views: [{ name: 'bound_form', data: { provider: 'object', object: 'contract' }, sections }],
159+
}).map((f) => `${f.rule}@${f.path}`)).toEqual([
160+
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[0]`,
161+
`${FORM_FIELD_UNKNOWN}@views[0].sections[0].fields[1]`,
162+
]);
124163
});
125164

126165
it('ignores non-form views (no sections array)', () => {
@@ -295,9 +334,12 @@ describe('#6251 — the view CONTAINER ladder', () => {
295334
form: { data: { object: 'contract' }, groups: [{ fields: ['name', { field: 'ghost_g', colSpan: 3 }] }] },
296335
}],
297336
});
337+
// [#17328] RE-JUDGED: the second row was the withdrawn
338+
// `absolute-colspan-discouraged` finding on the same entry. The `colSpan: 3`
339+
// is deliberately LEFT on the fixture — it is what proves the entry object
340+
// shape is read here at all — but it no longer earns a row.
298341
expect(findings.map((f) => `${f.rule}@${f.path}`)).toEqual([
299342
`${FORM_FIELD_UNKNOWN}@views[0].form.groups[0].fields[1]`,
300-
`${FORM_COLSPAN_ABSOLUTE}@views[0].form.groups[0].fields[1].colSpan`,
301343
]);
302344
});
303345

@@ -408,10 +450,13 @@ describe('#6251 — reachable on a REAL parsed app stack', () => {
408450

409451
it('reports every planted defect on that stack — this is the assertion #6251 exists for', () => {
410452
const { value } = quietly(() => cliTierFor(structuredClone(appShape)));
453+
// [#17328] RE-JUDGED: the third row was the withdrawn colSpan finding on
454+
// the `ghost_named` entry. The other two rows are the ones #6251 exists for
455+
// — one per container rung — and both still fire, so the traversal
456+
// assertion this test carries is undiminished.
411457
expect(validateFormLayout(value!).map((f) => `${f.rule}@${f.path}`)).toEqual([
412458
`${FORM_FIELD_UNKNOWN}@views[0].form.sections[0].fields[2]`,
413459
`${FORM_FIELD_UNKNOWN}@views[0].formViews.create.sections[0].fields[1]`,
414-
`${FORM_COLSPAN_ABSOLUTE}@views[0].formViews.create.sections[0].fields[1].colSpan`,
415460
]);
416461
});
417462

@@ -520,16 +565,26 @@ describe('#16168 — falls back to the container list binding', () => {
520565
})).toEqual([]);
521566
});
522567

523-
it('the colSpan rule is unconditional on the object binding — it already fired on a list-bound container before this fix', () => {
524-
// Measured for the changeset: `absolute-colspan-discouraged` sits OUTSIDE
525-
// the `known`-gated block (validate-form-layout.ts, the `(b)` comment) —
526-
// it needs only `entry.colSpan != null`, never `objName` / `known`. So it
527-
// was never actually dead on HotCRM's list-bound views; #16168 only fixes
528-
// `form-field-unknown` and `form-section-group-unknown`.
529-
const findings = validateFormLayout({
568+
// [#17328] RE-JUDGED, not deleted. This was #16168's measurement that
569+
// `absolute-colspan-discouraged` sat OUTSIDE the `known`-gated block — it
570+
// needed only `entry.colSpan != null`, so it was never dead on HotCRM's
571+
// list-bound views and #16168 only ever fixed the two reference rules. That
572+
// rule is now withdrawn, so what the same fixture measures is the OTHER half
573+
// of the same sentence: on a list-bound container a well-formed field with a
574+
// `colSpan` is silent, while a dangling one is not — which is exactly the
575+
// #16168 fix still holding, now with nothing riding alongside it.
576+
it('[#17328] a list-bound container reports only its reference findings — the colSpan rule is gone', () => {
577+
expect(validateFormLayout({
530578
objects,
531579
views: [listBoundContainer({ sections: [{ columns: 2, fields: [{ field: 'name', colSpan: 2 }] }] })],
532-
});
533-
expect(findings.map((f) => f.rule)).toEqual([FORM_COLSPAN_ABSOLUTE]);
580+
})).toEqual([]);
581+
582+
// POSITIVE CONTROL — the #16168 fix itself: the same list-bound rung, a
583+
// dangling reference, still reported. Without this the line above would
584+
// pass on a container the walk never descends into.
585+
expect(validateFormLayout({
586+
objects,
587+
views: [listBoundContainer({ sections: [{ columns: 2, fields: [{ field: 'ghost_lb', colSpan: 2 }] }] })],
588+
}).map((f) => f.rule)).toEqual([FORM_FIELD_UNKNOWN]);
534589
});
535590
});

0 commit comments

Comments
 (0)