mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(build): include shared UI stylesheets in Nx cache inputs (#1360)
* fix(build): include shared UI stylesheets in Nx cache inputs `libs/ui/styles` held shared SCSS partials but had no `project.json`, so its files belonged to no Nx project and were absent from every task hash. Editing a partial and running `pnpm nx build web` reported 4 of 4 tasks cached and shipped the previous CSS — a silent wrong build rather than a failure. Nx derives its project graph from TypeScript imports only, so a relative Sass `@use` that crosses a project root creates no edge. Verified directly: after adding the project but before declaring anything, `ui-styles` still had zero dependents in the graph. Make it the `ui-styles` project (no targets — it exists to be hashed) and declare `implicitDependencies` on the 8 consumers. Chosen over adding the path to `sharedGlobals`, which would put shared styles into every project's hash and make a one-line SCSS tweak mark the whole workspace affected. A styles edit now marks 15 projects affected and leaves electron-backend, website, the mock servers and the shared libs alone. `libs/ui/styles` was the only projectless directory holding files under `libs/` or `apps/`. Add `pnpm run styles:inputs:validate` to keep it closed: it resolves every relative stylesheet import against Nx's real project graph and fails when one escapes the input closure of a build that compiles it, naming the project to declare. It exits 1 with 21 diagnostics on the pre-fix tree. Imports of `apps/web/src/nav-list.scss` are deliberately accepted — `web` already hashes that file, and a lib -> app edge would make the graph cyclic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(build): spawn git without a shell in the stylesheet check `execSync("git ls-files '*.scss'")` runs through `cmd.exe` on Windows, where single quotes are literal characters rather than quoting. Git received the pathspec with the quotes intact, matched nothing and exited 0, so `styles:inputs:validate` reported success after checking zero stylesheets — silently disabling the check for Windows developers while staying green. Spawn with `execFileSync` so no shell is involved and git expands its own pathspec; verified to return the identical 133 files. Both this and the eslint glob trap next to it in the docs report success while covering nothing, so also make an empty scan fail rather than pass: the workspace always contains SCSS, and a listing that returns none means the scan broke. Reported by Codex review on #1360. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(styles): move nav-list partial into ui-styles (#1361) * fix(build): count every target of a comma-separated Sass @import `@import` is the only rule that takes a list, and the scan read just its first target. A later entry crossing an Nx project boundary escaped the cache key while the check still reported success — the same silent-pass failure the tool exists to prevent. Parse every target of an `@import` list. The obvious "read all quoted strings" fix trades one silent gap for a phantom one, so the rule decides: `@use`/`@forward` load exactly one module and a quoted string after it is `with (...)` configuration, and `url(...)` stays a plain CSS import the browser resolves at runtime. Neither is a module Sass compiles. The workspace has no relative `@import` at all today, so the scan still finds the same 42 imports across 133 files; this closes the gap before someone writes one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0010dd7351
commit
c741815b97
26 files changed
+537
-7
No files matched your search
@@ -37,7 +37,7 @@ Use it when changing existing views or introducing new list-based UI in the work
|
||||
- Shared EPG list styles:
|
||||
`libs/ui/epg/src/lib/epg-list-view/epg-list-view.component.scss`
|
||||
- Shared list selection style:
|
||||
`apps/web/src/nav-list.scss`
|
||||
`libs/ui/styles/_nav-list.scss`
|
||||
- Theme tokens:
|
||||
`apps/web/src/m3-theme.scss`
|
||||
- Settings surfaces:
|
||||
|
||||
@@ -116,6 +116,60 @@ For a buildable library that has a local `package.json`, its `name` must match
|
||||
the scoped alias. Nx uses that package name when rewriting buildable dependency
|
||||
paths to `dist/` during `@nx/js:tsc` builds.
|
||||
|
||||
## Shared Stylesheets and Cache Inputs
|
||||
|
||||
Nx derives the project graph from TypeScript imports. A relative Sass `@use`
|
||||
that crosses a project root creates **no** graph edge, so without an explicit
|
||||
declaration the imported partial belongs to no task's input set. The build then
|
||||
reports a cache hit for a stylesheet edit and serves the previous CSS — a
|
||||
silent wrong build rather than a failure.
|
||||
|
||||
Two rules keep that from happening:
|
||||
|
||||
1. A directory whose files are consumed by another project is itself an Nx
|
||||
project. Shared partials live in `libs/ui/styles`, project `ui-styles`,
|
||||
tagged `scope:shared`, `domain:shared-ui`, `type:ui`. It declares no targets;
|
||||
it exists so its files are hashed.
|
||||
2. Every consumer declares the dependency Nx cannot infer:
|
||||
|
||||
```json
|
||||
"implicitDependencies": ["ui-styles"]
|
||||
```
|
||||
|
||||
`@nx/enforce-module-boundaries` does not read stylesheets, so tag directions are
|
||||
not enforced here — keep consumers at `type:feature` or `type:ui`, both of which
|
||||
may depend on `type:ui`.
|
||||
|
||||
Importing a partial that the consuming **application** owns is a different case
|
||||
and needs no declaration, because that partial already sits inside the app's own
|
||||
build inputs. It is still the wrong direction, and it is the one case the two
|
||||
rules above cannot repair: a lib → app edge would make the graph cyclic, since
|
||||
the app already depends on those libraries. Move the partial into `ui-styles`
|
||||
instead. No library stylesheet imports from `apps/` today — keep it that way.
|
||||
|
||||
`pnpm run styles:inputs:validate` enforces both rules. It resolves every
|
||||
relative `@use`/`@forward`/`@import` in the workspace against Nx's own project
|
||||
graph and fails when an imported stylesheet sits outside the input closure of a
|
||||
build that compiles it, naming the project to declare. Comment-only example
|
||||
paths are ignored, so the documentation blocks inside the shared partials do not
|
||||
register as broken imports. CI runs it in the `unit-and-typecheck` job.
|
||||
|
||||
Only a module Sass actually compiles counts as an input. `@import` is the one
|
||||
rule that takes a comma-separated list, and **every** target in it is a separate
|
||||
dependency — reading just the first would let a later cross-project target
|
||||
escape the cache key while the check still passed. A quoted string after the
|
||||
module in `@use`/`@forward` belongs to a `with (...)` configuration and is a
|
||||
value, and `url(...)` stays a plain CSS import the browser resolves at runtime;
|
||||
neither is a build input, and treating either as one would report a phantom
|
||||
broken import.
|
||||
|
||||
Verify a suspected caching gap directly — add a comment to a partial, run the
|
||||
consuming build, and confirm the task runs instead of reporting a cache hit:
|
||||
|
||||
```bash
|
||||
pnpm nx build web --verbose
|
||||
```
|
||||
|
||||
## TypeScript File Size
|
||||
|
||||
`tools/eslint/max-lines-config.mjs` is the single source of truth:
|
||||
@@ -145,6 +199,14 @@ An unquoted `**` can expand to a shallow subset on POSIX while still returning
|
||||
success. After editing such a target, compare ESLint's linted-file count with
|
||||
the `find` count.
|
||||
|
||||
Repository tooling in `tools/` has the mirror-image trap: Node's `execSync`
|
||||
runs through `cmd.exe` on Windows, where single quotes are literal characters
|
||||
rather than quoting, so a POSIX-quoted pattern reaches the program intact and
|
||||
matches nothing. Spawn without a shell — `execFileSync('git', ['ls-files',
|
||||
'*.scss'])` — and let the program expand its own patterns. Both traps report
|
||||
success while covering nothing, so a check that scans an empty file set must
|
||||
fail rather than pass.
|
||||
|
||||
## CI Enforcement
|
||||
|
||||
The CI lint job runs affected projects on pull requests and all projects on
|
||||
|
||||
Reference in new issue
Block a user