refactor: migrate styling from scss to vanilla-extract#68
refactor: migrate styling from scss to vanilla-extract#68shubug1015 wants to merge 6 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR migrates the styling system from SCSS to Vanilla Extract, replacing global class names with type-safe, locally-scoped CSS modules. Build tooling is updated to use Vanilla Extract transformers, dependencies are swapped from SCSS to Vanilla Extract packages, and components are refactored to import and apply style objects instead of string class names. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @package.json:
- Around line 63-65: Update the three vanilla-extract dependency entries in
package.json to published versions to avoid npm install failures: change
"@vanilla-extract/css" version to "1.17.4", "@vanilla-extract/jest-transform" to
"1.1.17", and "@vanilla-extract/rollup-plugin" to "1.4.2" (replace the current
^1.17.5, ^1.1.19, ^1.5.0 entries respectively); run a fresh npm install and
lockfile update (npm install or yarn install) to regenerate
package-lock.json/yarn.lock and verify the project builds.
In @src/styles/animations.css.ts:
- Around line 79-87: The flip animations flipIn and flipOut hardcode
perspective(40rem); change this to use a pixel-based default and make it
customizable via a CSS variable (e.g., var(--flip-perspective, 600px)) so the
perspective uses a standard px unit and a reasonable default value; update both
flipIn and flipOut to reference that variable instead of 40rem and ensure the
variable name is documented/consistent with other style vars.
🧹 Nitpick comments (3)
src/types/animations.ts (1)
1-1: Consider exporting AnimationName from the public API.If consumers might benefit from referencing this type (e.g., for type guards or custom utilities), ensure it's re-exported from the main entry point.
#!/bin/bash # Description: Check if AnimationName is exported from the main entry point # Find the main entry point and check exports fd -t f 'index.ts' -x rg -n "export.*AnimationName"src/utils/generateAnimation/keyframesMap.ts (1)
4-35: Consider adding type safety for animation variant keys.The
keyframesMapuses plain strings for variant keys ('in', 'out', 'up', etc.), which could allow typos to slip through at compile time. Consider creating specific variant types for each animation category to improve type safety.🔧 Optional enhancement for stronger typing
Define variant types in
src/types/animations.ts:export type FadeVariant = 'in' | 'out'; export type SlideVariant = 'up' | 'down' | 'left' | 'right'; export type ScaleVariant = 'in' | 'out'; export type RotateVariant = 'clockwise' | 'counterclockwise'; export type BounceVariant = 'in' | 'out'; export type ElasticVariant = 'in' | 'out'; export type FlipVariant = 'in' | 'out'; export type AnimationVariants = { fade: FadeVariant; slide: SlideVariant; scale: ScaleVariant; rotate: RotateVariant; bounce: BounceVariant; elastic: ElasticVariant; flip: FlipVariant; };Then update the map type:
-export const keyframesMap: Record<AnimationName, Record<string, string>> = { +export const keyframesMap: { + [K in AnimationName]: Record<AnimationVariants[K], string> +} = { fade: { in: animations.fadeIn, out: animations.fadeOut, }, // ... rest of the map };src/styles/animations.css.ts (1)
43-51: Inconsistent CSS variable pattern in rotate animations.The rotate animations wrap the entire
rotate()function inside the CSS variable (e.g.,var(--rotate-from, rotate(0deg))), while other animations (slide, scale) only put the numeric value in the variable. This inconsistency makes the rotate animations harder to customize—users must provide the full transform function rather than just an angle value.🔧 Align rotate animations with the pattern used in other animations
export const rotateClockwise = keyframes({ - from: { transform: 'var(--rotate-from, rotate(0deg))' }, - to: { transform: 'var(--rotate-to, rotate(360deg))' }, + from: { transform: 'rotate(var(--rotate-from, 0deg))' }, + to: { transform: 'rotate(var(--rotate-to, 360deg))' }, }); export const rotateCounterclockwise = keyframes({ - from: { transform: 'var(--rotate-from, rotate(360deg))' }, - to: { transform: 'var(--rotate-to, rotate(0deg))' }, + from: { transform: 'rotate(var(--rotate-from, 360deg))' }, + to: { transform: 'rotate(var(--rotate-to, 0deg))' }, });
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
jest.config.tspackage.jsonrollup.config.jssrc/components/AnimatedSpan/AnimatedSpan.tsxsrc/components/NodeMotion/NodeMotion.tsxsrc/components/TextMotion/TextMotion.tsxsrc/styles/animations.css.tssrc/styles/animations.scsssrc/styles/motion.css.tssrc/styles/motion.scsssrc/types/animations.tssrc/utils/generateAnimation/generateAnimation.spec.tssrc/utils/generateAnimation/generateAnimation.tssrc/utils/generateAnimation/keyframesMap.tstsconfig.json
💤 Files with no reviewable changes (2)
- src/styles/motion.scss
- src/styles/animations.scss
🧰 Additional context used
🧬 Code graph analysis (2)
src/utils/generateAnimation/generateAnimation.ts (2)
src/utils/generateAnimation/keyframesMap.ts (1)
keyframesMap(4-35)src/types/animations.ts (1)
AnimationName(1-1)
src/utils/generateAnimation/keyframesMap.ts (1)
src/types/animations.ts (1)
AnimationName(1-1)
🔇 Additional comments (23)
rollup.config.js (1)
8-8: LGTM! Vanilla Extract plugin correctly integrated.The vanillaExtractPlugin is properly imported and added to the Rollup pipeline, enabling .css.ts file processing as part of the migration from SCSS.
Also applies to: 54-54
tsconfig.json (1)
16-16: Appropriate setting for Vanilla Extract imports.The
allowImportingTsExtensionsoption enables importing.css.tsfiles, which is necessary for the Vanilla Extract migration. However, note that the conventional approach is to import these files without the.tsextension (e.g.,from './styles.css'), letting the bundler resolve the.css.tsfile.jest.config.ts (2)
7-10: LGTM! Transform configuration correctly handles Vanilla Extract files.The transform configuration properly routes
.css.tsfiles through the Vanilla Extract Jest transformer while maintaining ts-jest for TypeScript files. This is the correct setup for testing components that import Vanilla Extract styles.
14-14: Appropriate exclusion of CSS files from coverage.Excluding
src/styles/**/*.css.tsfrom coverage collection is correct, as Vanilla Extract style files contain styling declarations rather than testable logic.src/components/AnimatedSpan/AnimatedSpan.tsx (2)
36-36: LGTM! CSS module class correctly applied.The
className={styles.motion}correctly applies the Vanilla Extract style to the animated span, properly replacing the previous SCSS-based styling approach.
3-3: The import pattern in this codebase is already correct and follows Vanilla Extract best practices. The.css.tsextension is the official Vanilla Extract convention for authoring style files, and the codebase consistently uses this import pattern across all files (4 instances, 100% uniform). While some toolchains optionally allow importing from.csswithout the.tssuffix, this project's bundler clearly requires the full.css.tspath, as evidenced by the absence of any.css-only imports in the codebase. Changing to the suggested.cssimport path could break the build if the resolver doesn't support stripping the.tsextension.Likely an incorrect or invalid review comment.
src/components/NodeMotion/NodeMotion.tsx (3)
7-7: LGTM! Vanilla Extract import added correctly.The import of Vanilla Extract styles replaces the previous SCSS imports, providing type-safe style references.
118-118: LGTM! Type-safe className application.The
styles.inanimatereference provides compile-time type safety and eliminates the risk of typos in class names.
111-111: The review concern is invalid—this is a new component, not a modification.The git history shows
NodeMotion.tsxis a new file being added, not an existing component being modified. The animatedTagelement was never assigned a className in the initial implementation, so there is no className being removed. The codebase contains no references to "node-motion" as a CSS class name.Likely an incorrect or invalid review comment.
src/utils/generateAnimation/generateAnimation.spec.ts (2)
163-163: LGTM! Correct test expectation for prototype filtering.The empty string expectation confirms that
generateAnimationcorrectly ignores enumerable properties from the prototype chain usinghasOwnPropertyfiltering.
5-22: Mock is complete and covers all animation exports fromanimations.css.ts.The jest.mock includes all 16 animation keyframes exported from the animations file (fadeIn, fadeOut, slideUp, slideDown, slideLeft, slideRight, scaleIn, scaleOut, rotateClockwise, rotateCounterclockwise, bounceIn, bounceOut, elasticIn, elasticOut, flipIn, flipOut). Non-animation exports (DELAY, DURATION, utility functions) are correctly excluded from the mock.
Likely an incorrect or invalid review comment.
src/components/TextMotion/TextMotion.tsx (2)
7-7: LGTM! Consistent Vanilla Extract migration.The import change matches the pattern used in
NodeMotion.tsx, ensuring consistency across components.
108-108: Clarify if className was actually removed from the animated element. The current code shows both TextMotion and NodeMotion follow an identical pattern: no className on the animated element (line 108), andstyles.inanimateon the non-animated element (line 115). If a className was previously applied to the animated element, confirm which class and whether external consumers depend on it.src/styles/motion.css.ts (2)
3-6: LGTM! Motion style supports text animation correctly.The
inline-blockdisplay enables CSS transforms, andwhiteSpace: 'pre'preserves spacing for text animation—both essential for the library's animation behavior.
8-10: No action needed. Thevisibility: 'hidden'implementation is correct for SSR and hydration scenarios.According to React 19 and SSR best practices,
visibility: hiddenpreserves layout space while hiding content, preventing layout shifts during hydration. This is the recommended approach overdisplay: none, which removes layout space and can cause CLS (Cumulative Layout Shift) when content is revealed. The code correctly applies this pattern.Likely an incorrect or invalid review comment.
src/types/animations.ts (1)
1-1: LGTM! Type safety improvement for animation names.The
AnimationNameunion type provides compile-time validation for animation lookups, preventing typos and improving developer experience.src/utils/generateAnimation/generateAnimation.ts (1)
3-5: LGTM!The new imports correctly support the transition to runtime keyframe lookup via the centralized
keyframesMap.src/styles/animations.css.ts (5)
3-11: LGTM!The fade animations are correctly defined with sensible CSS variable fallbacks for customization.
13-31: LGTM!The slide animations correctly use CSS variables within transform functions, with sensible directional defaults. The syntax is valid and well-supported.
33-41: LGTM!The scale animations are properly defined with sensible fallback values for subtle scaling effects.
53-63: LGTM!The bounce animations are well-structured with appropriate keyframe percentages and CSS variable fallbacks that create a pleasing bounce effect.
65-77: LGTM!The elastic animations correctly use horizontal scaling (
scaleX) with multi-step keyframes to create an elastic stretch effect. The CSS variable structure supports full customization.src/utils/generateAnimation/keyframesMap.ts (1)
1-1: No action required. The import correctly follows Vanilla Extract conventions by including the full.css.tsextension in the import path. This is necessary for Vanilla Extract's build-time processing and is the standard pattern documented in Vanilla Extract's official API.
| "@vanilla-extract/css": "^1.17.5", | ||
| "@vanilla-extract/jest-transform": "^1.1.19", | ||
| "@vanilla-extract/rollup-plugin": "^1.5.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@vanilla-extract/css latest stable version 2025 2026 npm
💡 Result:
Latest stable @vanilla-extract/css on npm: 1.17.4. Install: npm i @vanilla-extract/css. [1]
Sources
[1] npm package page for @vanilla-extract/css.
🌐 Web query:
@vanilla-extract/jest-transform latest stable version 2025 2026 npm
💡 Result:
Latest stable on the npm registry: @vanilla-extract/jest-transform v1.1.17 (published June 12, 2025). [1][2]
Sources:
- npm package page (version shown: 1.1.17). [1]
- Snyk version listing (1.1.17 — published 12 Jun 2025). [2]
🌐 Web query:
@vanilla-extract/rollup-plugin latest stable version 2025 2026 npm
💡 Result:
As of Jan 9, 2026: the upstream releases page shows @vanilla-extract/rollup-plugin v1.4.2 (released Nov 14, 2025), while the npm registry page currently lists v1.4.1 as the published package. [1][2]
Sources:
- GitHub releases (vanilla-extract): @vanilla-extract/rollup-plugin@1.4.2 (Nov 14, 2025). [1]
- npm package page: @vanilla-extract/rollup-plugin — version 1.4.1 (published ~a month ago). [2]
🌐 Web query:
vanilla-extract security vulnerabilities CVE 2025 2026
💡 Result:
Short answer: There are no CVEs specifically assigned to the vanilla-extract project (packages like @vanilla-extract/css, /vite-plugin, /webpack-plugin, /esbuild-plugin) in 2025–2026 that I can find. However, there was a related npm supply‑chain incident: the unrelated package docusaurus-plugin-vanilla-extract was reported as malicious following an npm account takeover (advisory dated November 24, 2025). Security scanners (Snyk / ReversingLabs) report no known security issues for the official vanilla-extract packages. [1][2][3]
Sources:
- GitLab advisory — docusaurus-plugin-vanilla-extract (supply‑chain malware, Nov 24, 2025). [1]
- Snyk package pages for vanilla-extract plugins (no direct vulnerabilities reported). [2]
- ReversingLabs / package scans for @vanilla-extract/css (no vulnerabilities detected). [3]
If you want, I can (a) search NVD/CVE databases for any newly published CVEs right now, or (b) list recommended mitigation steps (dependency scanning, lockfile pinning, rotate credentials) for supply‑chain incidents.
Update Vanilla Extract package versions—specified versions do not exist in npm registry.
The package.json references versions that have not been released:
- @vanilla-extract/css: ^1.17.5 (latest is 1.17.4)
- @vanilla-extract/jest-transform: ^1.1.19 (latest is 1.1.17)
- @vanilla-extract/rollup-plugin: ^1.5.0 (latest is 1.4.2)
These will cause npm install to fail. Update to available versions: 1.17.4, 1.1.17, and 1.4.2 respectively. No security vulnerabilities were found in the official vanilla-extract packages.
🤖 Prompt for AI Agents
In @package.json around lines 63 - 65, Update the three vanilla-extract
dependency entries in package.json to published versions to avoid npm install
failures: change "@vanilla-extract/css" version to "1.17.4",
"@vanilla-extract/jest-transform" to "1.1.17", and
"@vanilla-extract/rollup-plugin" to "1.4.2" (replace the current ^1.17.5,
^1.1.19, ^1.5.0 entries respectively); run a fresh npm install and lockfile
update (npm install or yarn install) to regenerate package-lock.json/yarn.lock
and verify the project builds.
| export const flipIn = keyframes({ | ||
| from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 90deg))' }, | ||
| to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 0deg))' }, | ||
| }); | ||
|
|
||
| export const flipOut = keyframes({ | ||
| from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 0deg))' }, | ||
| to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 90deg))' }, | ||
| }); |
There was a problem hiding this comment.
Perspective value uses unusual unit and may be too large.
The flip animations use perspective(40rem) which has two concerns:
-
Unusual unit: Perspective is typically specified in
pxunits, notrem. Usingremmakes the perspective scale with the root font size, which could lead to inconsistent 3D effects across different devices or user font preferences. -
Large value: 40rem is extremely large (640px at default 16px font size). Standard perspective values for UI animations are typically 400-600px. A larger perspective creates a more subtle 3D effect, which may be intentional but should be documented or made customizable.
-
Not customizable: Unlike other animation parameters, the perspective value is hardcoded and cannot be adjusted via CSS variables.
🎨 Proposed fix to use standard units and add customization
export const flipIn = keyframes({
- from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 90deg))' },
- to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 0deg))' },
+ from: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-from, 90deg))' },
+ to: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-to, 0deg))' },
});
export const flipOut = keyframes({
- from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 0deg))' },
- to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 90deg))' },
+ from: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-from, 0deg))' },
+ to: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-to, 90deg))' },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const flipIn = keyframes({ | |
| from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 90deg))' }, | |
| to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 0deg))' }, | |
| }); | |
| export const flipOut = keyframes({ | |
| from: { transform: 'perspective(40rem) rotateY(var(--flip-from, 0deg))' }, | |
| to: { transform: 'perspective(40rem) rotateY(var(--flip-to, 90deg))' }, | |
| }); | |
| export const flipIn = keyframes({ | |
| from: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-from, 90deg))' }, | |
| to: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-to, 0deg))' }, | |
| }); | |
| export const flipOut = keyframes({ | |
| from: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-from, 0deg))' }, | |
| to: { transform: 'perspective(var(--flip-perspective, 400px)) rotateY(var(--flip-to, 90deg))' }, | |
| }); |
🤖 Prompt for AI Agents
In @src/styles/animations.css.ts around lines 79 - 87, The flip animations
flipIn and flipOut hardcode perspective(40rem); change this to use a pixel-based
default and make it customizable via a CSS variable (e.g.,
var(--flip-perspective, 600px)) so the perspective uses a standard px unit and a
reasonable default value; update both flipIn and flipOut to reference that
variable instead of 40rem and ensure the variable name is documented/consistent
with other style vars.
| const keyframe = keyframesMap[name as AnimationName]?.[variant]; | ||
|
|
||
| if (keyframe) { | ||
| const animationString = `${keyframe} ${duration}s ${easing} ${calculatedDelay}s both`; | ||
| acc.animationStrings.push(animationString); | ||
| } |
There was a problem hiding this comment.
Unsafe type assertion could break with future animation types.
The type assertion name as AnimationName on line 41 assumes that any non-'custom' key with a 'variant' property is a valid AnimationName. While this holds true for the current codebase, adding new animation types without updating the type definition could cause runtime lookup failures that are silently ignored by the conditional on line 43.
Additionally, when a keyframe is not found (line 43), the code silently skips it without any warning. This makes it difficult for consumers to debug misconfigurations or typos in animation names/variants.
🛡️ Proposed fix to add type guard and development warning
+ const isValidAnimationName = (key: string): key is AnimationName => {
+ return ['fade', 'slide', 'scale', 'rotate', 'bounce', 'elastic', 'flip'].includes(key);
+ };
+
if (name === 'custom') {
const { name: animationName, duration, delay, easing = 'ease-out' } = config as CustomAnimation;
const calculatedDelay = sequenceIndex * delay + initialDelay;
const animationString = `${animationName} ${duration}s ${easing} ${calculatedDelay}s both`;
acc.animationStrings.push(animationString);
} else if ('variant' in config) {
const { variant, duration, delay, easing = 'ease-out', ...rest } = config;
const calculatedDelay = sequenceIndex * delay + initialDelay;
- const keyframe = keyframesMap[name as AnimationName]?.[variant];
+ const keyframe = isValidAnimationName(name) ? keyframesMap[name]?.[variant] : undefined;
if (keyframe) {
const animationString = `${keyframe} ${duration}s ${easing} ${calculatedDelay}s both`;
acc.animationStrings.push(animationString);
+ } else if (process.env.NODE_ENV !== 'production') {
+ console.warn(`Animation keyframe not found for: ${name}.${variant}`);
}Committable suggestion skipped: line range outside the PR's diff.
| "resolveJsonModule": true, | ||
| "esModuleInterop": true, | ||
| "skipLibCheck": true, | ||
| "noEmit": true, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the tsconfig.json file to understand the full context
cat -n tsconfig.jsonRepository: shubug1015/react-textmotion
Length of output: 786
🏁 Script executed:
# Check for any Rollup configuration files
fd -e "rollup" -e "rollup.config" -e "js" -e "ts" | grep -i rollup | head -20Repository: shubug1015/react-textmotion
Length of output: 87
🏁 Script executed:
# Look for build configuration files
ls -la | grep -E "(rollup|webpack|vite|build)"Repository: shubug1015/react-textmotion
Length of output: 138
🏁 Script executed:
cat -n rollup.config.jsRepository: shubug1015/react-textmotion
Length of output: 1830
🏁 Script executed:
# Also check package.json to understand the build script
cat -n package.jsonRepository: shubug1015/react-textmotion
Length of output: 4082
Confusing TypeScript configuration: noEmit: true conflicts with declaration: true in tsconfig.json.
The tsconfig.json has noEmit: true (Line 15) alongside declaration: true (Line 8) and outDir (Line 7), which are logically contradictory. While this works in practice because the Rollup TypeScript plugin explicitly overrides these settings with declaration: true and declarationDir: 'dist' (rollup.config.js, lines 40-41), the configuration is unnecessarily confusing and could cause issues in other contexts (IDE type checking, manual tsc runs, or future tooling changes).
The noEmit: true appears intentional for the test:type script (package.json, line 39), but this purpose should be clearer in the configuration.
Consider restructuring to reduce confusion:
- Move
noEmitto a separate tsconfig for type-checking purposes, or - Add a comment explaining that Rollup's plugin overrides these settings for the build.
1 participant
Description
This PR refactors the existing SCSS-based styling system to Vanilla Extract in order to make
react-textmotionmore suitable as a reusable library.Key improvements include:
No changes were made to the public API or component behavior.
Related Issue
Closes #67
Type of change
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.