ReviewOS

stacks/lastsoulultra

Internal links are written two ways with nothing enforcing either

#25
Open glennmichael123 opened this 4 days ago ·
4 days ago

Internal navigation is written two ways with no rule separating them, so some links navigate client side and some do a full page load, and which you get depends on who wrote the line.

Full sweep of resources/**/*.stx in hq.training, 330 templates:

StxLink                332
internal <a href>       49
external                 9
in-page #hash           10
mailto / tel             3
dynamic expression     180

So StxLink is overwhelmingly the house idiom and the 49 are the exception.

It is not a policy

The same destinations are reached both ways, in the same application:

/coaches/athletes is an StxLink in components/AccountHome.stx, components/CoachAthleteBrowser.stx, components/coach-library.stx, views/today.stx and views/import.stx, and a plain anchor in views/coaches/index.stx (twice) and components/SettingsWebsite.stx.

/profile is an StxLink in components/AccountHome.stx and components/app-nav.stx (twice), and a plain anchor in views/metrics.stx.

Five files use both forms within the same file:

fileinternal anchorsStxLink
views/[username].stx61
views/profile.stx42
views/coaches/index.stx25
components/SettingsWebsite.stx26
views/metrics.stx12

and 22 more use internal anchors with no StxLink at all.

It is not unmigrated legacy either

StxLink has been present since the original "convert to stacks app" commit, and 138 commits have touched it since. The files still carrying anchors were last edited on 2026-10-15, 2026-10-29 and 2026-10-30. This is current code being written this way now, not old code nobody has got to.

The actual cause

Nothing enforces it. There is no rule in the repository's lint configuration, and pickier does not ship one. Every link is whatever its author typed that day, and both forms work, so nothing ever corrects it.

Suggested fix

A lint rule is worth more here than a sweep. A one-off conversion of the 49 will drift again, which the October edits above already demonstrate. Something along the lines of "an <a> whose href begins with / inside a template should be an StxLink", with an allowance for the deliberate exceptions:

  • the footer language switcher (components/LanguageSwitch.stx), whose anchors are query-string and call preventDefault anyway, with the real href kept for middle-click and for people without JS
  • genuinely cross-shell links, if any are intended to force a full load

Then convert the 49 in one pass behind the rule.

Related: #24 covers workspace-nav.stx specifically, which is the one navigation component entirely on anchors. This issue is the general case.

Correcting my own earlier count in #24: a rough grep gave 392 and 250. Parsing properly, excluding external, hash, mailto and dynamic hrefs, it is 332 and 49. The conclusion in #24 is unchanged.

4 days ago

One refinement, from trying to act on #24 and finding I had it wrong.

I proposed a rule along the lines of "an <a> whose href begins with / should be an StxLink". That is not safe as stated, because a computed href may resolve to another origin at runtime.

resources/components/workspace-nav.stx is the case in point. Its links are :href="workspaceUrl(link.to)", and workspaceUrl (resources/functions/workspace-brand.ts:15) returns brand.settings.home_url, privacy_url, terms_url, or a URL built on home_url for several paths, and a same-origin path for the rest. Which one you get depends on brand settings, not on anything visible at the call site. StxLink would break the external cases, so the anchors there are correct. I closed #24 accordingly.

So the rule wants to be narrower than I first wrote:

  • flag a literal internal href: <a href="/coaches/athletes"> and the like, which is where all five mixed files and the real drift live
  • leave :href="expr" alone entirely, since the value is not knowable statically
  • keep the exemption for LanguageSwitch.stx, whose anchors are query-string and call preventDefault

That still covers the cases this issue is actually about. Of the 49 internal anchors counted, the literal ones are the ones worth converting, and the dynamic ones were never the problem.

Sign in to comment on this issue.