Skip to content

feat(oxlint): add AppShell lint plugin - #536

Merged
IzumiSy merged 12 commits into
mainfrom
feat/oxlint-plugin-app-shell
Sep 16, 2026
Merged

IzumiSy merged 12 commits into
mainfrom
feat/oxlint-plugin-app-shell

Conversation

@IzumiSy

@IzumiSy IzumiSy commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

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 recommended Oxlint config so consumers enable the initial rules by extending one shared object from oxlint.config.ts.

Stable consumer boundaries

The preset rejects direct imports from react-router and react-router-dom, plus the internal astw: 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.gap remains the documented column-spacing API.

Summary

  • Add the publishable @tailor-platform/eslint-plugin-app-shell package.
  • Add no-react-router-imports and no-astw-prefix under the @tailor-platform/app-shell namespace.
  • Update consumer patterns and component guidance to remove astw: usage.
  • Cover direct-rule behavior and the consumer oxlint.config.ts integration with tests.
  • Add release changesets for the plugin and AppShell guidance update.

Related issues

  • tailor-inc/platform-planning#1651Remove astw: prefix advice from public docs and the bundled skill. This PR covers the bundled-skill half (catalogue/** guidance) and adds the no-astw-prefix rule 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 — so tailor-inc/platform-planning#1651 is only fully resolved once both land.
  • Community report behind it: tailor-professional-service/knowledge discussion 342 — https://github.com/tailor-professional-service/knowledge/discussions/342. A consumer found astw:-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.

@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (297f7e0) #536 (b45889a) +/-
Coverage 87.3% 87.3% 0.0%
Test Execution Time 1m23s 1m55s +32s
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%)

Files Coverage +/- Patch Coverage Status
packages/eslint-plugin/package.json 100.0% 0.0% 100.0% added

Reported by octocov

@IzumiSy
IzumiSy marked this pull request as ready for review September 15, 2026 04:56
@IzumiSy
IzumiSy requested a review from a team as a code owner September 15, 2026 04:56
@@ -0,0 +1,50 @@
{
"name": "@tailor-platform/eslint-plugin-app-shell",
"version": "0.0.0",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

interacsean added a commit that referenced this pull request Sep 16, 2026
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>
interacsean added a commit that referenced this pull request Sep 16, 2026
#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>
interacsean added a commit that referenced this pull request Sep 16, 2026
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>
interacsean added a commit that referenced this pull request Sep 16, 2026
`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>
Comment thread catalogue/src/fundamental/components.md Outdated

- **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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Table.Head/Table.Cell carry first:pl-6/last:pr-6

So the containerClassName should not need a px-6

Comment thread catalogue/src/fundamental/components.md Outdated
**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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above (does not need containerClassName="px-6")

Comment thread catalogue/src/fundamental/components.md Outdated
```tsx
<Card.Root>
<Table.Root containerClassName="astw:px-6">{/* … */}</Table.Root>
<Table.Root containerClassName="px-6">{/* … */}</Table.Root>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove containerClassName="px-6"

Comment thread catalogue/src/fundamental/components.md Outdated
<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>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove containerClassName="px-6"

Comment thread catalogue/src/fundamental/components.md Outdated
Comment on lines 483 to 489
**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>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is now the correct form

@interacsean interacsean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@IzumiSy

IzumiSy commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Copilot AI and others added 4 commits September 16, 2026 04:47
…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>

Copilot AI commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in b89b674. I also fixed the post-merge catalogue validation follow-ups in 44b3bc1, 681ed6b, and 3ce43fd.

@IzumiSy
IzumiSy merged commit ee90145 into main Sep 16, 2026
8 checks passed
@IzumiSy
IzumiSy deleted the feat/oxlint-plugin-app-shell branch September 16, 2026 06:45
interacsean added a commit that referenced this pull request Sep 16, 2026
`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>
interacsean added a commit that referenced this pull request Sep 16, 2026
#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>
interacsean added a commit that referenced this pull request Sep 17, 2026
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>
interacsean added a commit that referenced this pull request Sep 17, 2026
…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>
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.

3 participants