feat(oxlint): add AppShell lint plugin - #536
Conversation
Code Metrics Report
Details | | main (297f7e0) | #536 (b45889a) | +/- |
|---------------------|----------------|----------------|------|
| Coverage | 87.3% | 87.3% | 0.0% |
| Files | 198 | 198 | 0 |
| Lines | 5931 | 5931 | 0 |
| Covered | 5182 | 5182 | 0 |
- | Test Execution Time | 1m23s | 1m55s | +32s |Code coverage of files in pull request scope (100.0% → 100.0%, patch 100.0%)
Reported by octocov |
| @@ -0,0 +1,50 @@ | |||
| { | |||
| "name": "@tailor-platform/eslint-plugin-app-shell", | |||
| "version": "0.0.0", | |||
There was a problem hiding this comment.
It looks like npm allows us to start 0.0.0
I will release this package locally first, and get on the track of our usual release process using changesets.
PR #536 supersedes this branch's guidance for the bundled skill, so its version wins on every file the two touch: catalogue/src/fundamental/{components,design-system}.md catalogue/src/pattern/detail/hero-with-actions/{PATTERN.md,*.tsx} catalogue/src/pattern/form/composer/PATTERN.md packages/core/src/components/layout/types.ts Theirs is the better line: rather than teaching a specificity escape hatch, it removes the need for one — table-in-card renders Table.Root directly after the card header instead of overriding Card.Content's padding, and Layout.gap stays a supported prop rather than being deprecated toward a className override. It also ships the rule as lint (@tailor-platform/eslint-plugin-app-shell, no-astw-prefix). examples/nextjs-app was deleted upstream (#513, #532) in favour of the Vite showcase, so its four conflicting files are dropped. Git matched the rest by rename, carrying this branch's conversions into examples/vite-app/src/{pages/showcase/{colors,grid,metric-card},showcase/form-demo}.tsx — including the items-stretch! fix from the adversarial-review round. Retained from this branch: the #1652 prefix-order fix in four components, its regression guard, docs/**, and the ActionPanel JSDoc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#536 sets the line for the bundled skill but does not touch docs/, which would otherwise keep recommending the prefix. Same rule, now everywhere: ordinary utilities on your own markup and on documented layout hooks; props, variants, or composition for a component's own appearance. This drops the `!` importance-modifier rule this branch had documented. Theirs is better — it removes the need for the override rather than teaching a specificity hack — so each site moves to the supported form: Layout gap-6! -> gap={6} (prop, un-deprecated by #536) Layout p-8! -> dropped (the root owns its padding) Button justify-start! -> dropped Button text-destructive! -> variant="destructive" DescriptionCard rounded-lg! shadow-md! -> dropped (card owns its surface) Badge px-3! py-1! / text-lg! -> dropped (Badge owns its metrics) styling-theming.md keeps the measurement as *explanation* rather than advice: it says why a plain utility cannot override an AppShell default, which is what sends you to a prop, and points at the new no-astw-prefix lint rule. Also converts surfaces #536 does not reach: - docs/components/ai-chat.md (15 sites) — landed on main after this branch and carried the same advice - the Vite showcase's ai-chat, layouts and primitives pages; layouts also had two misordered `dark:astw:opacity-20`, fixed by the same strip - form-demo's Field.Root stretch now uses the inline style the row already positions itself with, instead of `items-stretch!` Menu has no destructive variant and its `data-highlighted` colour outranks an unprefixed utility, so the one destructive Menu.Item uses an inline style. Verified: it holds the destructive colour while highlighted where a utility class flips to accent-foreground. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main moved ahead of #536 with #514, which relocated catalogue/src/pattern/detail/hero-with-actions/ to page/detail/ and corrected the table-in-card rule. The two disagree on substance, so "prefer #536" does not settle this one: main is right and #536 would regress it. Measured in examples/vite-app, insets from the card's left edge: #536 Card.Header + containerClassName="px-6" title 25px, column 49px #514 Card.Content className="astw:px-0" title 25px, column 25px this no Card.Content, containerClassName="pb-6" title 25px, column 25px Table.Head and Table.Cell already carry first:pl-6 / last:pr-6 (table.tsx:122,138), so container padding stacks and pushes the first column 24px past the card title — exactly what #514 says. Resolved to main's geometry expressed without the prefix, which is the only form that satisfies both PRs: render Table.Root directly inside Card.Root (or directly after Card.Header) rather than wrapping it in Card.Content, and let the cells supply the horizontal inset. That drops #536's containerClassName="px-6" and main's astw:px-0 alike. The DON'T example now shows the double-padding case. Also accepts main's deletion of the pattern/ copies, relocated by #514. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`astw:` is the library's internal Tailwind prefix. A consumer's Tailwind build has no `astw` prefix configured, so it never generates `astw:*` classes — an `astw:` class in app code only resolves if app-shell happens to already ship that exact utility. 17 of 55 classes sampled from these pages are absent from the shipped stylesheet, and Tailwind emits nothing for an unknown utility: no error, no warning. Adopts the boundary #536 sets for the bundled skill, so the two agree once it lands: ordinary utilities on your own markup and on documented layout hooks; props, variants, or composition for a component's own appearance. Each site moved to the supported form — `Layout gap={6}`, `Button variant="destructive"`, and so on — rather than to a specificity hack. styling-theming.md keeps the measurement as explanation rather than advice: it says why a plain utility cannot override an AppShell default, which is what sends you to a prop, and points at #536's no-astw-prefix lint rule. Deliberately leaves catalogue/ and Layout's `gap` JSDoc to #536 so the two branches do not overlap. Table-in-card guidance follows #514: the cells carry their own first:pl-6 / last:pr-6, so the container adds no horizontal padding. Measured in examples/vite-app — with `containerClassName="px-6"` the first column sits at 49px against a card title at 25px; without it, both at 25px. Also converts the Vite showcase's ai-chat, layouts and primitives pages (layouts also carried two misordered `dark:astw:opacity-20`), and ActionPanel's JSDoc, which ships in the published .d.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
||
| - **Inside a card?** Pass `containerClassName="astw:px-6"` on `Table.Root` for the horizontal inset, and either drop `Card.Content` (bare list form) or pass `Card.Content className="astw:px-0"` (header+content form). Skipping the `containerClassName` lands the first column flush against the card edge. See the `Card` entry for the two canonical forms and a DON'T example. Dense cell typography (**`text-sm`**, **`tabular-nums`** for figures, **`font-mono`** for identifiers) → **`design-system.md`** §4 Typography. | ||
| - **Whole row is clickable.** Use `<Table.Row onClick={() => navigate(detailPath)} className="astw:cursor-pointer">`. For keyboard and screen-reader users, also wrap the primary identifier cell content in `<Link>` (so the row is reachable via Tab; `Table.Row` is a `<tr>` and cannot itself be a Link — wrapping a `<tr>` in `<a>` is invalid HTML). **No per-row "View" / "Open" / "→" buttons.** Per-row `Menu` (overflow `…`) is the only allowed per-row action surface and is reserved for non-navigation actions like Archive, Duplicate. | ||
| - **Inside a card?** Pass `containerClassName="px-6"` on `Table.Root` for the horizontal inset. When the card has a header, render `Table.Root` directly after it and add `pb-6` to the container classes. Skipping the horizontal inset lands the first column flush against the card edge. See the `Card` entry for the canonical forms and a DON'T example. Dense cell typography (**`text-sm`**, **`tabular-nums`** for figures, **`font-mono`** for identifiers) → **`design-system.md`** §4 Typography. |
There was a problem hiding this comment.
Table.Head/Table.Cell carry first:pl-6/last:pr-6
So the containerClassName should not need a px-6
| **Notes:** | ||
|
|
||
| - **Tables inside a card need TWO co-requisite geometry changes** (token-backed spacing rationale → **`design-system.md`** §4 Spacing): (a) Card stops imposing horizontal padding — drop `Card.Content` for the bare form, or pass `Card.Content className="astw:px-0"` for the header+content form. (b) `Table.Root` provides the inset itself via `containerClassName="astw:px-6"`. Skipping (b) lands the first column flush against the card edge — `Table.Cell`'s intrinsic `astw:first:pl-6` does NOT render reliably in this composition. | ||
| - **Tables inside a card need a container inset** (token-backed spacing rationale → **`design-system.md`** §4 Spacing): `Table.Root` provides it via `containerClassName="px-6"`. When a card header precedes the table, render the table directly after it and add `pb-6` to preserve the card's bottom inset. Skipping the horizontal inset lands the first column flush against the card edge. |
There was a problem hiding this comment.
As above (does not need containerClassName="px-6")
| ```tsx | ||
| <Card.Root> | ||
| <Table.Root containerClassName="astw:px-6">{/* … */}</Table.Root> | ||
| <Table.Root containerClassName="px-6">{/* … */}</Table.Root> |
There was a problem hiding this comment.
Remove containerClassName="px-6"
| <Card.Content className="astw:px-0"> | ||
| <Table.Root containerClassName="astw:px-6">{/* … */}</Table.Root> | ||
| </Card.Content> | ||
| <Table.Root containerClassName="px-6 pb-6">{/* … */}</Table.Root> |
There was a problem hiding this comment.
Remove containerClassName="px-6"
| **DON'T — first column lands flush against the card edge:** | ||
|
|
||
| ```tsx | ||
| <Card.Root> | ||
| <Card.Content className="astw:px-0"> | ||
| <Table.Root>{/* missing containerClassName="astw:px-6" */}</Table.Root> | ||
| </Card.Content> | ||
| <Card.Header title="Line items" /> | ||
| <Table.Root>{/* missing containerClassName="px-6" */}</Table.Root> | ||
| </Card.Root> |
There was a problem hiding this comment.
This section is now the correct form
interacsean
left a comment
There was a problem hiding this comment.
My new comments were pre-existing issues so ok if we want to address separately
| "@tailor-platform/app-shell": patch | ||
| --- | ||
|
|
||
| Stop documenting the `astw:` prefix as an application customization API. It is an internal prefix for AppShell's precompiled CSS, so consumer-authored utilities may be absent from the shipped stylesheet and couple applications to component internals. |
There was a problem hiding this comment.
I've been trying to reconcile the side-work here on fixing the astw: prefix documentation issue with side-work that one of my tickets is doing
Over on my branch there are extra instances of this, can you point your Claude to:
https://github.com/tailor-platform/app-shell/pull/515/changes
and see if any of these should be brought in?
Or is there an intent to isolate the blast radius of the fix for the astw prefix to only some relevant files?
There was a problem hiding this comment.
Thanks for checking. Since this PR is focused on the lint plugin, I’m planning to merge it first without pulling in the broader astw: documentation/examples cleanup.
Could you please continue with the remaining cleanup and prefix-order fixes in #515? That should keep the broader follow-up in one place and avoid duplicating work here.
|
@copilot resolve the merge conflicts in this pull request |
…p-shell # Conflicts: # catalogue/src/fundamental/components.md # catalogue/src/fundamental/design-system.md # catalogue/src/pattern/detail/hero-with-actions/PATTERN.md # catalogue/src/pattern/detail/hero-with-actions/hero-with-actions.tsx Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
`astw:` is the library's internal Tailwind prefix. A consumer's Tailwind build has no `astw` prefix configured, so it never generates `astw:*` classes — an `astw:` class in app code only resolves if app-shell happens to already ship that exact utility. 17 of 55 classes sampled from these pages are absent from the shipped stylesheet, and Tailwind emits nothing for an unknown utility: no error, no warning. Adopts the boundary #536 sets for the bundled skill, so the two agree once it lands: ordinary utilities on your own markup and on documented layout hooks; props, variants, or composition for a component's own appearance. Each site moved to the supported form — `Layout gap={6}`, `Button variant="destructive"`, and so on — rather than to a specificity hack. styling-theming.md keeps the measurement as explanation rather than advice: it says why a plain utility cannot override an AppShell default, which is what sends you to a prop, and points at #536's no-astw-prefix lint rule. Deliberately leaves catalogue/ and Layout's `gap` JSDoc to #536 so the two branches do not overlap. Table-in-card guidance follows #514: the cells carry their own first:pl-6 / last:pr-6, so the container adds no horizontal padding. Measured in examples/vite-app — with `containerClassName="px-6"` the first column sits at 49px against a card title at 25px; without it, both at 25px. Also converts the Vite showcase's ai-chat, layouts and primitives pages (layouts also carried two misordered `dark:astw:opacity-20`), and ActionPanel's JSDoc, which ships in the published .d.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#536 landed reconciled against #514, and its merged form differs from the branch this docs pass was written against: it keeps `Card.Content className="px-0!"` for the one table-in-card case rather than restructuring around it. Docs that never mention `!` would contradict the shipped skill. Props, variants and composition stay the primary guidance, matching §5. `!` is now documented as the last resort it is in the catalogue, using the catalogue's own example, and with the note that the table container still adds no horizontal padding — the cells carry first:pl-6 / last:pr-6, so `containerClassName="px-6"` there would push the first column 24px past the card title. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback: this belongs in linting, not in the test suite. Removed rather than left in place, so the gap is momentary instead of becoming tech debt. Backlogged as an oxlint rule. It is not a one-line config change: #536's no-astw-prefix is the consumer-facing rule and would flag every legitimate use inside packages/core, that plugin is not wired into this repo's own lint, and oxlint 1.64 ships no generic restricted-syntax rule that reaches inside class strings. The board item also notes #1700 may remove the prefix entirely and make the rule moot. The 14 fixes in this PR stand on their own; nothing else in the suite covers the defect, since snapshots record class names rather than resolved CSS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…prefix advice in docs/ (#515) * fix(core): move astw: to the front of 14 misordered utility classes (#1652) Tailwind v4 only recognises a configured prefix as the first segment of a class name, so `hover:astw:opacity-100` and friends emitted no CSS at all — no error, no warning. 14 such classes across Alert, MetricCard, AppearanceSwitcher and CsvImporter were dead in the shipped stylesheet. Verified against the built CSS: all 14 selectors are present by exact-string match where none were before, and confirmed in the browser — Alert's per- variant muted description text and its dismiss focus ring (previously no focus indicator at all), MetricCard's dark-mode deltas, the appearance switcher's radio indicator, and CsvImporter's last-row border. Adds src/astw-prefix-order.test.ts as a guard — snapshots record class names rather than resolved CSS, so they stay green on this defect. It checks that `astw:` occurs exactly once and at position 0, scans .css too (globals.css drives five `@apply astw:*` rules), and names the boundaries it cannot cover. Each evasion was probed before being closed. Also fixes the one misordered class outside packages/core, in decisions/date-picker.md, and records the ordering rule in the internal component-authoring reference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(core): remove astw: prefix advice from the public docs (#1651) `astw:` is the library's internal Tailwind prefix. A consumer's Tailwind build has no `astw` prefix configured, so it never generates `astw:*` classes — an `astw:` class in app code only resolves if app-shell happens to already ship that exact utility. 17 of 55 classes sampled from these pages are absent from the shipped stylesheet, and Tailwind emits nothing for an unknown utility: no error, no warning. Adopts the boundary #536 sets for the bundled skill, so the two agree once it lands: ordinary utilities on your own markup and on documented layout hooks; props, variants, or composition for a component's own appearance. Each site moved to the supported form — `Layout gap={6}`, `Button variant="destructive"`, and so on — rather than to a specificity hack. styling-theming.md keeps the measurement as explanation rather than advice: it says why a plain utility cannot override an AppShell default, which is what sends you to a prop, and points at #536's no-astw-prefix lint rule. Deliberately leaves catalogue/ and Layout's `gap` JSDoc to #536 so the two branches do not overlap. Table-in-card guidance follows #514: the cells carry their own first:pl-6 / last:pr-6, so the container adds no horizontal padding. Measured in examples/vite-app — with `containerClassName="px-6"` the first column sits at 49px against a card title at 25px; without it, both at 25px. Also converts the Vite showcase's ai-chat, layouts and primitives pages (layouts also carried two misordered `dark:astw:opacity-20`), and ActionPanel's JSDoc, which ships in the published .d.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(core): match the merged catalogue on the `!` override escape #536 landed reconciled against #514, and its merged form differs from the branch this docs pass was written against: it keeps `Card.Content className="px-0!"` for the one table-in-card case rather than restructuring around it. Docs that never mention `!` would contradict the shipped skill. Props, variants and composition stay the primary guidance, matching §5. `!` is now documented as the last resort it is in the catalogue, using the catalogue's own example, and with the note that the table container still adds no horizontal padding — the cells carry first:pl-6 / last:pr-6, so `containerClassName="px-6"` there would push the first column 24px past the card title. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(core): drop the astw: prefix-order guard Review feedback: this belongs in linting, not in the test suite. Removed rather than left in place, so the gap is momentary instead of becoming tech debt. Backlogged as an oxlint rule. It is not a one-line config change: #536's no-astw-prefix is the consumer-facing rule and would flag every legitimate use inside packages/core, that plugin is not wired into this repo's own lint, and oxlint 1.64 ships no generic restricted-syntax rule that reaches inside class strings. The board item also notes #1700 may remove the prefix entirely and make the rule moot. The 14 fixes in this PR stand on their own; nothing else in the suite covers the defect, since snapshots record class names rather than resolved CSS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Seiya IZUMI <IzumiSy@users.noreply.github.com>
Motivation
AppShell consumer guidance needs executable feedback for its routing boundary and styling boundary. Direct React Router imports bypass AppShell’s router integration, while the internal
astw:CSS prefix was incorrectly documented as a consumer customization API.Design Decision
Low-friction configuration
Provide a
recommendedOxlint config so consumers enable the initial rules by extending one shared object fromoxlint.config.ts.Stable consumer boundaries
The preset rejects direct imports from
react-routerandreact-router-dom, plus the internalastw:prefix anywhere in application source. It does not autofix router imports because not every router API is part of AppShell’s public barrel.The pattern catalogue now uses ordinary Tailwind utilities only for explicitly documented layout hooks. Table-in-card examples render the table directly after the card header, avoiding an internal-padding override;
Layout.gapremains the documented column-spacing API.Summary
@tailor-platform/eslint-plugin-app-shellpackage.no-react-router-importsandno-astw-prefixunder the@tailor-platform/app-shellnamespace.astw:usage.oxlint.config.tsintegration with tests.Related issues
tailor-inc/platform-planning#1651— Removeastw:prefix advice from public docs and the bundled skill. This PR covers the bundled-skill half (catalogue/**guidance) and adds theno-astw-prefixrule that enforces it. The public-docs half (docs/**) is owned by fix(core): correct astw: prefix order in shipped components, and the prefix advice in docs/ #515, which is deliberately scoped to have zero file overlap with this branch — sotailor-inc/platform-planning#1651is only fully resolved once both land.tailor-professional-service/knowledgediscussion 342 — https://github.com/tailor-professional-service/knowledge/discussions/342. A consumer foundastw:-prefixed utilities silently emitting no CSS, because the prefix was documented as something consumer code could use when it is build-time-internal to AppShell. The lint rule added here is what turns that from tribal knowledge into feedback at the point of use.Note:
tailor-inc/platform-planning#1652(misordered prefix dropping 14 utility classes in shipped components) is a separate, code-level defect handled entirely in #515; nothing in this PR addresses it.