Add CSS bundler for slashed-essential.css - #5
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR hardens the CSS bundler script with path validation and improved error handling, then integrates it into the git workflow via a pre-commit hook and npm automation to automatically build and stage bundles on commit. ChangesCSS Bundler Hardening and Git Integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.githooks/pre-commit:
- Around line 1-3: The pre-commit hook can succeed even if bundling fails
because it doesn't stop on errors; update the hook so the script exits
immediately on any failure by either adding a shell "set -e" at the top of the
script or chaining the commands with "&&" (e.g., run "node scripts/bundle.js &&
git add dist/slashed-essential.css"); ensure you modify the existing pre-commit
script that runs node scripts/bundle.js and git add dist/slashed-essential.css
so a failing bundle prevents the git add and the hook from succeeding.
In `@package.json`:
- Line 6: The prepare script currently runs "git config core.hooksPath
.githooks" unconditionally which can fail when not in a Git worktree; update the
prepare entry so it first checks for a Git repo (e.g., using git rev-parse
--is-inside-work-tree or testing for a .git directory) and only runs git config
core.hooksPath .githooks when that check succeeds; modify the "prepare" script
in package.json to perform that guarded check before invoking git config.
In `@scripts/bundle.js`:
- Around line 22-24: The bundle currently injects a wall-clock timestamp via the
timestamp const and embeds it into header (and the similar header occurrences
around the other two spots), causing nondeterministic output; remove the
Date().toISOString() call and stop including timestamp in the header strings
(replace with a static, deterministic header message or a reproducible build
id), i.e., remove the timestamp variable and change the const header (and the
other header definitions at the two similar spots) to use only deterministic
content such as `/* ${path.basename(output)} — bundled */\n`.
- Line 21: The code trusts config paths (e.g., output used with path.join to
form outputPath) and can be tricked to read/write outside the repo; fix by
resolving and validating each config-derived path against the repository root
before any file IO: use path.resolve(ROOT, <configPath>) (instead of raw
path.join), then ensure the resolved path is constrained to ROOT (e.g.,
path.relative(ROOT, resolved) does not start with '..' and the resolved path
shares the ROOT prefix) and reject or normalize any paths that escape the root;
apply this validation to outputPath and the other config-derived paths used
around the same area (the variables/functions that build paths from the parsed
bundle.config.json).
- Around line 9-16: loadConfig currently calls process.exit(1) on JSON
read/parse errors which kills watch mode; change loadConfig to catch errors, log
the error (including err.message), and return null/undefined or a safe default
instead of exiting so the process survives transient failures (keep CONFIG_PATH
and loadConfig name to locate the change). Also update the file-watcher handler
that currently reacts only to 'change' (referenced around the code handling
fs.watch events / lines 79-83) to also handle the 'rename' event: on 'rename'
attempt to re-read loadConfig (with retries or a short debounce), trigger a
rebuild/reload when config becomes available, and avoid throwing so watch mode
continues running. Ensure rebuild logic uses the returned config-null check
rather than relying on process exit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20c2a6fb-5dff-44fd-8fab-127388551e94
⛔ Files ignored due to path filters (2)
dist/slashed-essential.cssis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.githooks/pre-commit.gitignorebundle.config.jsonpackage.jsonscripts/bundle.js
Removed dist/ from .gitignore so the generated bundle is part of the repository — users can grab the CSS directly without a build step. https://claude.ai/code/session_01BYukqam6ayV3B9K1icXiET
Runs the bundler before every commit and stages dist/slashed-essential.css automatically. Hook lives in .githooks/ (tracked by git) and is activated via core.hooksPath — run once after cloning: git config core.hooksPath .githooks https://claude.ai/code/session_01BYukqam6ayV3B9K1icXiET
Added prepare script so git config core.hooksPath .githooks runs automatically after npm install — no manual setup needed after cloning. https://claude.ai/code/session_01BYukqam6ayV3B9K1icXiET
- pre-commit: add set -e so a failed bundle blocks the commit - package.json: guard prepare with git rev-parse to avoid failures outside git repos - bundle.js: remove timestamp from header for deterministic output - bundle.js: add resolveInsideRoot() to validate config paths against ROOT - bundle.js: loadConfig() now throws instead of process.exit so watch mode survives errors - bundle.js: handle rename event on config watcher (atomic-save editors) https://claude.ai/code/session_01BYukqam6ayV3B9K1icXiET
64e9a7a to
f505bb9
Compare
Resolves the 10 concerns surfaced in the design-level review of PR #130 (stored at semantic-review/2026-05-27-233854-pr-130.md, gitignored). #1 Migrate-mode silent data loss on existing-class collision - Added validateMigrate() pre-validation pass that runs BEFORE any mutation. Detects three cases: a) Target class doesn't exist → safe (creates with seed). b) Target class exists, no key overlap → safe (additive merge). c) Target class exists, same key with different value → CONFLICT. Hard-errors before any mutation, with a message naming the conflicting keys and suggesting Add mode or a different name. - Per-op merge logic: when target exists with no conflicts, missing seed keys are added to its settings before upsert. upsertGlobalClass still never overwrites — we only ADD keys it didn't have. The 'never overwrite' policy is preserved (overwrite = replacing existing values; this is purely additive). - End result: migrate is now safe in all three cases. removeMigratedKeys is only called after every key is guaranteed to be provided by the (new or existing) class with the same value. #2 Modifier-mode auto-numbering rejected legitimate multi-row applies - Two siblings each producing card__image--lg is the canonical 'attach this modifier to all of them' case, NOT a collision. Replaced the blanket name.includes('--') reject with a per-mode check: in modifier mode, intra-plan duplicates are intentional, no error and no numbering. upsertGlobalClass dedupes by name so all rows share the single class. #3 + #10 'label' provenance not honored as authoritative - Added AUTHORITATIVE_PROVENANCE = new Set(['user', 'label']) and use it consistently in applyAutoNumbering. Both user-typed and structure-panel-label-derived names are treated as authoritative and never auto-renumbered. Two authoritative rows colliding is a hard error (mentions row count, not provenance details, since the user shouldn't need to know about reBEMer's internal vocabulary). - Updated apply.js JSDoc typedef to enumerate all five provenance values: 'user' | 'label' | 'element-type' | 'fallback' | 'auto-number'. - §0 status table no longer overstates this row. #4 Auto-numbering didn't re-check post-numbered names - Added a final post-numbering integrity pass: in non-modifier modes, every op must have a unique finalClass after numbering. Catches the case where a user-typed card__image-1 collides with an auto-numbered card__image-1 from a different group. Modifier mode is exempt (duplicates are by design). #5 Badge dataset flag never cleared on host disconnect - Removed the ATTACHED_FLAG dataset entirely. refreshBadges() now uses badgeInstances map state as the source of truth, with two explicit passes: reap stale (host disconnected), then mount any <li> without a live host inside it. The li.contains(existing.host) check handles the common Bricks pattern of rerendering inner .structure-item without removing the outer <li>. - Eliminates the permanent-skip bug where a row that was rendered once and then had its inner subtree rebuilt by Bricks would lose its badge forever. #6 'Use existing' hint not migrate-aware - Row.svelte recommendation copy now branches on mode. Migrate-mode copy reflects the new validateMigrate semantics: 'On Apply, missing style keys will be merged into it. Conflicting values block the migration — pick a different name or use Add.' - Non-migrate modes keep the original 'attach the existing class instead of creating a duplicate' copy. #7 Auto-numbered names didn't refresh the 'use existing' hint - Refactored apply.js to expose a pure buildPlan({ rootId, rows, mode }) → { ok, ops, error }. Returns the ops with their POST-numbering finalClass values. - BemPanel.svelte computes a previewClassNames Map that runs buildPlan and indexes the result by row id. Reactive — re-runs when rows or mode change. - Row.svelte now accepts finalClassName as a prop (from the panel's preview map) instead of computing candidateClassName locally. The 'use existing' hint matches what apply.js will actually produce, even after auto-numbering. - Same buildPlan is used by applyToSubtree, so preview and apply can never disagree. #8 Unused-classes scan was non-deterministic past the cap - Added ORDER BY meta_id ASC to the postmeta query in class-rebemer-rest.php. The cap+1 truncation detection now pairs with reproducible content: two truncated runs on the same site return the same first 5000 rows. #9 Apply has no mid-apply rollback (acknowledged spec-only) - Snapshot/rollback transactional apply (§10) remains spec-only, but the partial-failure path now (a) console.warn-logs the failure with the count of completed mutations, and (b) surfaces a partial-success toast: 'Applied to N elements before halting: <error>. State is partially applied — undo via Bricks (Cmd-Z) before retrying.' The user has actionable info instead of a bare error. Also addresses cleanup - Added semantic-review/ to .gitignore so future review artifacts from the sub-agent don't accidentally land in commits. Build artifacts regenerated. Bundle grew from 60.20 kB to 62.25 kB JS (+0.6 kB gzip) — the new buildPlan refactor + validateMigrate pre-pass + post-numbering check. Acceptable for the correctness gain. What's still spec-only after this PR Items #4 and #6 in §0 status table are now ✅ shipped where they were partial; the remaining ❌ rows are unchanged (cross-page preflight, snapshot/rollback, undo ring buffer, i18n, reserved-name guard wired from inventory).
- Fix #1: use normalizedQuery (trimmed) in template search branch so whitespace-only input doesn't switch to the full 275-token view - Fix #2: return applyToColorInput() result from onPickValue callback; show error toast and keep picker open if application fails - Fix #3: simplify toggleFamily() to always reset alphaOpen to a new Set(), preventing stale open-alpha state when switching families - Fix #4: reorder getBricksPanelRight() selectors — panel-specific IDs first, drop broad builder-root selectors (#brx-builder, .brx-builder) - Fix #5: add :focus-visible rules for all new interactive elements (.slashed-cp__qu-cell, .slashed-cp__scan-row, .slashed-cp__fam-banner, .slashed-cp__strip-sw, .slashed-cp__alpha-btn) - Fix #6: change .slashed-cp__strip-sw border from 0 to 1px solid transparent so --alpha dashed override actually renders https://claude.ai/code/session_01HesqASnoEx3Amm4H7f17jj
Summary
scripts/bundle.js— bundler konkatenujący pliki zcore/w prawidłowej kolejności cascade layers dodist/slashed-essential.cssbundle.config.json— dodanie nowego pliku nie wymaga edycji skryptu.githooks/pre-commit) automatycznie przebudowuje bundle przed każdym commitempreparewpackage.jsonaktywuje hook automatycznie ponpm install— zero ręcznej konfiguracjiUżycie
Dodanie nowego pliku core
Wystarczy dopisać go w odpowiednim miejscu w
bundle.config.json— przy następnym commicie bundle zaktualizuje się automatycznie.https://claude.ai/code/session_01BYukqam6ayV3B9K1icXiET
Generated by Claude Code
Summary by CodeRabbit