feat: migrate deprecated classes and custom properties#4338
feat: migrate deprecated classes and custom properties#4338Blackbaud-ErikaMcVey merged 10 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Angular migration schematic and tests that scan workspace source files to replace deprecated CSS classes and custom properties using mappings derived from imported design-token/style JSON; includes type definitions, migrations manifest entry, JSON import support, and dependency bumps. Changes
Sequence DiagramsequenceDiagram
participant CLI as User/CLI
participant Schematic as Migration Schematic
participant Styles as Styles JSON
participant Tokens as Tokens JSON
participant Workspace as Workspace Config
participant Files as Source Files
CLI->>Schematic: run migration
Schematic->>Styles: import styles JSON
Styles-->>Schematic: provide style groups & deprecated classes
Schematic->>Tokens: import tokens JSON
Tokens-->>Schematic: provide token groups & deprecated properties
Schematic->>Schematic: build replacement maps
Schematic->>Workspace: read projects and sourceRoot
Workspace-->>Schematic: return project roots
Schematic->>Files: iterate supported files & read contents
Files-->>Schematic: return file content
Schematic->>Schematic: apply boundary-aware replacements
alt file changed
Schematic->>Files: write updated content
Files-->>Schematic: confirm write
Schematic->>CLI: log "Updated <filePath>"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 806911b
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts`:
- Around line 102-115: applyCustomPropertyReplacements currently does a raw
global string replace which causes substring matches (e.g., --sky-old-var
matching --sky-old-var-extended); update the matching logic in
applyCustomPropertyReplacements to use a boundary-aware RegExp similar to the
class replacement path (use lookarounds or explicit token boundaries around the
custom property name) so only exact custom property names are replaced, and add
a regression test that covers a suffixed property (e.g., --sky-old-var and
--sky-old-var-extended) to ensure only the exact name is changed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 685de262-bfca-4c33-8b5c-1d041d13cb57
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
libs/components/packages/migrations.jsonlibs/components/packages/package.jsonlibs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.tslibs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.tslibs/components/packages/tsconfig.json
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
|
Component Storybooks:
Apps: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts (1)
85-122: Extract the shared boundary-aware replacement loop.
applyClassReplacements()andapplyCustomPropertyReplacements()now have identical logic. Pulling that loop into one helper would keep future matcher changes from drifting between the two paths.♻️ Suggested simplification
+function applyBoundaryAwareReplacements( + content: string, + replacements: Record<string, string>, +): string { + let updated = content; + + for (const [oldValue, newValue] of Object.entries(replacements)) { + const pattern = new RegExp( + `(?<![\\w-])${escapeRegExp(oldValue)}(?![\\w-])`, + 'g', + ); + updated = updated.replace(pattern, newValue); + } + + return updated; +} + function applyClassReplacements( content: string, replacements: Record<string, string>, ): string { - let updated = content; - - for (const [oldClass, newClass] of Object.entries(replacements)) { - const pattern = new RegExp( - `(?<![\\w-])${escapeRegExp(oldClass)}(?![\\w-])`, - 'g', - ); - updated = updated.replace(pattern, newClass); - } - - return updated; + return applyBoundaryAwareReplacements(content, replacements); } @@ function applyCustomPropertyReplacements( content: string, replacements: Record<string, string>, ): string { - let updated = content; - - for (const [oldProp, newProp] of Object.entries(replacements)) { - const pattern = new RegExp( - `(?<![\\w-])${escapeRegExp(oldProp)}(?![\\w-])`, - 'g', - ); - updated = updated.replace(pattern, newProp); - } - - return updated; + return applyBoundaryAwareReplacements(content, replacements); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts` around lines 85 - 122, Both applyClassReplacements and applyCustomPropertyReplacements contain the same boundary-aware loop; extract that loop into a single helper (e.g., applyBoundaryAwareReplacements or replaceWithBoundaryAwarePattern) that accepts content and replacements and returns the updated string, and then have applyClassReplacements and applyCustomPropertyReplacements call that helper with their respective replacements; preserve the existing regex construction using escapeRegExp and the lookaround (`(?<![\\w-])... (?![\\w-])`) and keep behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts`:
- Around line 85-122: Both applyClassReplacements and
applyCustomPropertyReplacements contain the same boundary-aware loop; extract
that loop into a single helper (e.g., applyBoundaryAwareReplacements or
replaceWithBoundaryAwarePattern) that accepts content and replacements and
returns the updated string, and then have applyClassReplacements and
applyCustomPropertyReplacements call that helper with their respective
replacements; preserve the existing regex construction using escapeRegExp and
the lookaround (`(?<![\\w-])... (?![\\w-])`) and keep behavior identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 91ebe3eb-ff4e-4cb1-8c6e-941f76390e0b
📒 Files selected for processing (2)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.tslibs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
...update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
Outdated
Show resolved
Hide resolved
...e-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts (1)
2-3: Consider pinning the replacement tables for this migration.Because this rule lives under
update-14, deriving its maps from the current design-token bundles makes the rewrite set drift with that package. Ifpublic-api-styles.json/public-api-tokens.jsonare not guaranteed to retain every historical alias across future releases, freezing the generated maps locally would make this migration deterministic for later upgrade paths.Also applies to: 63-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts` around lines 2 - 3, The migration currently imports live replacement maps via stylesJson and tokensJson from '@blackbaud/skyux-design-tokens/bundles/public-api-styles.json' and public-api-tokens.json which makes the rule non-deterministic; replace those dynamic imports by embedding pinned, versioned replacement tables as local constants (e.g., a JSON snapshot assigned to a const like pinnedStylesMap and pinnedTokensMap) inside replace-deprecated-css-vars-and-classes.ts so the migration uses fixed maps for update-14; update any references to stylesJson/tokensJson to use the pinned constants and add a brief comment noting the source/version used for the snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts`:
- Around line 407-408: The assertions in the overlap test are checking the wrong
token names: update the two expectations that reference '.sky-old-color' and
'--sky-old-color' to assert the actual pre-migration names used in the fixture
('.sky-color' and '--sky-color') so the `updated` output is validated against
the real old tokens; locate the two expect(updated).not.toContain(...) lines in
replace-deprecated-css-vars-and-classes.spec.ts and change the string literals
accordingly.
---
Nitpick comments:
In
`@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts`:
- Around line 2-3: The migration currently imports live replacement maps via
stylesJson and tokensJson from
'@blackbaud/skyux-design-tokens/bundles/public-api-styles.json' and
public-api-tokens.json which makes the rule non-deterministic; replace those
dynamic imports by embedding pinned, versioned replacement tables as local
constants (e.g., a JSON snapshot assigned to a const like pinnedStylesMap and
pinnedTokensMap) inside replace-deprecated-css-vars-and-classes.ts so the
migration uses fixed maps for update-14; update any references to
stylesJson/tokensJson to use the pinned constants and add a brief comment noting
the source/version used for the snapshot.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69862e2d-2271-4eba-ada2-d5b215e12e79
📒 Files selected for processing (2)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.tslibs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts
...e-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Adds an ng update migration under @skyux/packages to automatically replace deprecated SKY UX CSS class names and custom properties across project source files, driven by the design-tokens “public API” JSON bundles.
Changes:
- Bump
@blackbaud/skyux-design-tokensto5.5.0and add it as a dependency of@skyux/packages. - Add a new update-14 migration (
replace-deprecated-css-vars-and-classes) that scans workspace projects and performs boundary-aware string replacements in.html/.ts/.js/.css/.scss. - Enable TypeScript JSON imports for the packages project and add a comprehensive schematic test suite covering success cases and edge cases.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Bumps design-tokens dependency to support the new migration inputs. |
| package-lock.json | Locks the updated design-tokens version and updated metadata. |
| libs/components/theme/package.json | Keeps theme package aligned with updated design-tokens. |
| libs/components/packages/package.json | Adds design-tokens dependency needed by the migration at runtime. |
| libs/components/packages/tsconfig.json | Enables resolveJsonModule so schematics can import JSON bundles. |
| libs/components/packages/migrations.json | Registers the new update-14 migration entry. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.ts | Implements the traversal + replacement logic and workspace file scanning. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts | Adds tests for replacement behavior, boundaries, multi-project scanning, and fallback behaviors. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-token.ts | Defines the token JSON shape used for traversal. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-token-group.ts | Defines token group JSON shape used for traversal. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-tokens.ts | Defines root token JSON shape used for traversal. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-style.ts | Defines style JSON shape used for traversal. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-style-group.ts | Defines style group JSON shape used for traversal. |
| libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-styles.ts | Defines root style JSON shape used for traversal. |
...migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-style-group.ts
Outdated
Show resolved
Hide resolved
...tics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-tokens.ts
Outdated
Show resolved
Hide resolved
...migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-token-group.ts
Outdated
Show resolved
Hide resolved
...tics/migrations/update-14/replace-deprecated-css-vars-and-classes/types/public-api-styles.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts (1)
36-69:⚠️ Potential issue | 🟠 MajorExercise the real schematic in these integration tests.
Line 39 only proves the migration does not throw, and Lines 56-61 bypass the registered schematic entirely by calling
buildReplaceRule()with hand-written mappings. This suite would still stay green ifmigrations.json, the default factory, or the JSON-derived replacement maps broke and the real multi-project migration became a no-op. Seed one known deprecated class/custom property from the imported token/style JSON and runrunner.runSchematic(SCHEMATIC_NAME, {}, tree)for the multi-project path too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts` around lines 36 - 69, The multi-project test currently bypasses the real schematic by calling buildReplaceRule directly; change the test to seed one known deprecated token/class from the real imported JSON into the test trees (same strings used in your design token/style JSON) and invoke the actual schematic runner via runner.runSchematic(SCHEMATIC_NAME, {}, tree) (or the string literal used by your collection) instead of calling buildReplaceRule, then assert the transformed files contain the expected new class/property; keep createTestLibrary, COLLECTION_PATH and runSchematic/runner references to locate the test setup and replace the manual call accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts`:
- Around line 36-69: The multi-project test currently bypasses the real
schematic by calling buildReplaceRule directly; change the test to seed one
known deprecated token/class from the real imported JSON into the test trees
(same strings used in your design token/style JSON) and invoke the actual
schematic runner via runner.runSchematic(SCHEMATIC_NAME, {}, tree) (or the
string literal used by your collection) instead of calling buildReplaceRule,
then assert the transformed files contain the expected new class/property; keep
createTestLibrary, COLLECTION_PATH and runSchematic/runner references to locate
the test setup and replace the manual call accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a9b5b374-e3aa-477e-9e87-84731fdc03ea
📒 Files selected for processing (1)
libs/components/packages/src/schematics/migrations/update-14/replace-deprecated-css-vars-and-classes/replace-deprecated-css-vars-and-classes.spec.ts
Summary by CodeRabbit
New Features
Tests
Chores