Skip to content

Remove dead code surfaced by the automations removal sweep - #339

Merged
paulocastellano merged 6 commits into
mainfrom
chore/remove-dead-code
Sep 8, 2026
Merged

paulocastellano merged 6 commits into
mainfrom
chore/remove-dead-code

Conversation

@paulocastellano

@paulocastellano paulocastellano commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Closes #334.

Two commits: the dead-code removal itself, and a one-line fix to BrowserTestCase without which the browser suite fails locally for reasons that have nothing to do with this PR — see the second half.


1. The removal

Every file here has zero importers on main, verified by grepping the import path or class name across resources/js, app, config, routes, tests and resources/views.

125 files deleted, −3073 lines.

Removed

Frontend — leftovers of the Jan 2026 starter kit:
AlertError, AppContent, AppShell, Breadcrumbs, Heading, Icon, NavFooter, PlaceholderPattern, UserInfo, WorkspaceSwitcher, composables/useDateMaska.ts, layouts/GuestLayout.vue, layouts/auth/AuthCardLayout.vue, layouts/auth/AuthSimpleLayout.vue.

components/ui/* — 18 shadcn directories nothing ever imported:
alert-dialog, aspect-ratio, button-group, carousel, context-menu, drawer, empty, hover-card, input-group, input-otp, kbd, native-select, navigation-menu, progress, radio-group, resizable, scroll-area, tags-input.

Any of them comes back with npx shadcn-vue add <name> if it is ever wanted.

PHP — app/Concerns/PasswordValidationRules.php, app/Enums/Ai/Orientation.php, app/Exceptions/Ai/QuotaExhaustedException.php.

npm — axios, embla-carousel-vue, vue-input-otp, vaul-vue, maska.

Three corrections to the issue's list

ui/sheet kept. ui/sidebar/Sidebar.vue:4 imports Sheet/SheetContent for the mobile drawer, and Sidebar is rendered by AppSidebar.vue:207 → AppSidebarLayout.vue:43. Removing it would break the sidebar on phones.

ui/range-calendar kept. ui/date-range-picker/DateRangePicker.vue:18 uses it, and the picker is on pages/analytics/Index.vue:18.

Both slipped through because the issue's criterion was "no importer outside ui/" — a dependency from one ui/ directory to another still counts.

posts/previews/LinkCard.vue kept. It is live in XPreview.vue, ThreadsPreview.vue and BlueskyPreview.vue. The original sweep only matched single-quoted imports and that one is double-quoted.

One addition

resources/js/components/ui/input/InputMask.vue was not on the list but is an orphan too — exported from ui/input/index.ts, imported by nobody. It was the last thing keeping maska alive, so it goes with useDateMaska.ts.

Notes

  • Ai\Orientation was unused because the image pipeline passes raw 'portrait' / 'landscape' strings (AiImageClient.php:30, TemplateImageGenerator.php:175, UnsplashService.php:36). Deleting it matches the issue; adopting it in those three call sites instead would also be defensible — happy to do that in a follow-up if you prefer.
  • axios stays in node_modules as a transitive dependency of @inertiajs/vue3 (via @inertiajs/core and laravel-precognition). Only our direct declaration goes away, and nothing in resources/js ever imported it.
  • The issue's last note about 32 import/order errors is already resolved: npx eslint . is clean on main.

Audit

Check Result
Residual references to any removed file (whole repo) only generic-name false positives (Empty, Progress, Carousel, Icon)
Imports by path the one hit is @/components/ui/input, which still exists and exports Input
Kebab-case template usage (<alert-dialog>, <hover-card>) none
Empty directories left behind none
i18n keys orphaned by the removal none
Types orphaned by the removal none — the orphans in types/ already existed on main
npm ls for the four removed packages empty tree

2. Why BrowserTestCase changes here

Without the second commit, php artisan test tests/Browser fails locally on any machine with npm run dev running — three tests, none of them related to this PR, all failing on main too. Shipping the removal next to that would make it look like the deletions broke something.

tests/BrowserTestCase says in its own docblock that these tests load the built Vite assets, but $fakesVite = false only turns off the manifest fake. Laravel's Vite helper still prefers public/hot whenever that file exists — and it exists on every machine running npm run dev. So the browser tests were loading the app from the Vite dev server, not from the build.

That alone would be tolerable. What makes it fail is the interaction with i18n:

  • resources/js/app.ts:63 resolves translations lazily with import.meta.glob('../../lang/*.json').
  • lang/php_*.json is generated by the laravel-vue-i18n/vite plugin and gitignored (.gitignore:6).
  • The plugin deletes those files when a build finishes. Run npm run build while npm run dev is up and the dev server's runtime glob resolves nothing.

The page then renders raw translation keys. From the failure screenshot: auth.login.title, auth.login.email, auth.legal. So assertVisible('@legal-links') passed (the div is there) while assertSeeLink('Terms of Service') could not find the links, and RepurposeAccountHealthTest saw the literal repurposes.health.source_missing instead of the banner sentence.

FAILED  AuthLegalLinksTest > the login screen shows the legal sentence to a logged out visitor
FAILED  AuthLegalLinksTest > the register screen shows the legal sentence to a logged out visitor
FAILED  RepurposeAccountHealthTest > a repurpose whose source was deleted explains itself instead of rendering a hole

CI has no hot file, so it always used the manifest and never saw any of this.

The fix:

protected function setUp(): void
{
    parent::setUp();

    Vite::useHotFile(base_path('tests/.vite-hot-file-that-never-exists'));
}

Pointing the hot file at a path that can never exist makes the browser tests use the manifest unconditionally — what the class already claimed to do, and what CI has been doing all along. No effect in CI, where the hot file is absent either way.

This works because the HTTP server the browser plugin runs lives in the test process: Pest\Browser\Drivers\LaravelHttpServer::handleRequest() resolves the kernel out of the same container, so a setUp() override reaches the rendered page.

BrowserTestCase is only extended by tests/Browser (tests/Pest.php:32), so nothing else is touched.


Verification

Check Result
npx vue-tsc --noEmit exit 0
npx eslint . exit 0
npm run build exit 0
npm run build:ssr (used by docker/Dockerfile) exit 0
vendor/bin/pint --dirty passed
php artisan test --parallel 4390 passed, 1 skipped
php artisan test tests/Browser 43 passed (40 passed / 3 failed before the second commit)

The browser run is the one that matters for a PR that deletes 18 UI directories, and until the BrowserTestCase fix it was silently exercising the dev server instead of the build. With the fix it exercises the real built assets, without the removed packages, and is green.

Paulo Castellano added 2 commits September 7, 2026 17:18
Closes #334.

Everything here had zero importers on main, verified by grepping for the
import path or class name across resources/js, app, config, routes, tests
and resources/views.

Frontend: the leftovers of the Jan 2026 starter kit (AlertError, AppContent,
AppShell, Breadcrumbs, Heading, Icon, NavFooter, PlaceholderPattern,
UserInfo, WorkspaceSwitcher, GuestLayout, AuthCardLayout, AuthSimpleLayout,
useDateMaska) plus 18 shadcn ui/ directories nothing ever imported.

ui/sheet and ui/range-calendar stay, against what #334 listed: sheet backs
the mobile drawer in ui/sidebar/Sidebar.vue, which AppSidebar renders, and
range-calendar backs ui/date-range-picker, used by the analytics page. The
"no importer outside ui/" criterion missed those transitive edges.

posts/previews/LinkCard.vue also stays; it is live in the X, Threads and
Bluesky previews. The sweep's grep only covered single-quoted imports and
that one is double-quoted.

ui/input/InputMask.vue was not on the list but is an orphan too, and it was
the last thing holding maska.

PHP: PasswordValidationRules, the Ai\Orientation enum (the image pipeline
passes raw 'portrait'/'landscape' strings instead) and QuotaExhaustedException.

npm: axios, embla-carousel-vue, vue-input-otp, vaul-vue and maska drop out of
package.json. axios stays in the tree as a transitive dependency of
@inertiajs/vue3, it just is not ours to declare any more.
…ning

tests/BrowserTestCase already says these tests load the built Vite assets,
but it only turned off the manifest fake. Laravel still prefers public/hot
when it exists, so on any machine with `npm run dev` running the browser
tests silently loaded the app from the Vite dev server instead of the build
that `npm run build` had just produced.

That is enough on its own to make three tests fail locally while CI, which
has no hot file, stays green. The dev server resolves
`import.meta.glob('../../lang/*.json')` lazily at runtime, and the
laravel-vue-i18n Vite plugin deletes lang/php_*.json when a build finishes,
so the page rendered raw translation keys -- "auth.legal" instead of the
sentence with the Terms of Service and Privacy Policy links, and the raw
repurposes.health.source_missing key instead of the banner.

Pointing the hot file at a path that can never exist makes the browser tests
use the manifest unconditionally, which is what they claim to do and what CI
has been doing all along.

    AuthLegalLinksTest        the login screen shows the legal sentence
    AuthLegalLinksTest        the register screen shows the legal sentence
    RepurposeAccountHealthTest  a repurpose whose source was deleted ...

The HTTP server the browser plugin runs lives in the test process
(Pest\Browser\Drivers\LaravelHttpServer resolves the kernel out of the same
container), so a setUp() override reaches the rendered page.
Paulo Castellano added 4 commits September 7, 2026 23:05
The sweep removed 108 files under resources/js/components/ui because nothing
imported them. That reasoning does not hold there: those are the shadcn-vue
primitives, kept as a library to build from rather than as application code, so
being unimported is their normal state, not evidence they are dead. The
directory is now byte-identical to main again.

Also merges main, which brought the locale work in (#341). The two test files
both branches touched — WebhookPausedMailTest and WebhookTranslationsTest —
combined without conflict.
This commit cleans up the BrowserTestCase by eliminating the unused Vite facade and the associated setup method, which was previously intended to configure a hot file that never existed. The class now focuses solely on its core functionality without unnecessary dependencies.
The sweep dropped five packages as unused, but four of them are imported by the
primitives under resources/js/components/ui — restoring those files without the
packages broke the build, which is what the e2e job hit: "Rolldown failed to
resolve import maska/vue".

vaul-vue (drawer), vue-input-otp and embla-carousel-vue (carousel) come back:
if the primitives stay as a library to build from, so do the packages they
depend on. axios and maska stay out — nothing imports either, maska only
mattered for InputMask.vue, which is gone.

Also drops the Vite hot-file override from BrowserTestCase. It forced the tests
onto public/build even with the dev server running, so locally they read
whatever was last built rather than the code under test.
@paulocastellano
paulocastellano merged commit 3032866 into main Sep 8, 2026
5 checks passed
@paulocastellano
paulocastellano deleted the chore/remove-dead-code branch September 8, 2026 02:29
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.

Remove pre-existing dead code surfaced by the automations removal sweep

1 participant