fix(theme): replace cross-package relative style imports with package… - #2039
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates relative Less imports to package-name imports (e.g., using @delon/abc) across various theme files and adds the dist directory to the Less compiler paths in the build script. However, importing theme-default.less directly inside packages/abc/index.less breaks multi-theme support and should be avoided. Additionally, in the build script, the dist/ directory path should be ordered before node_modules/ to ensure that Less resolves package imports from the newly built files rather than stale node_modules.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -1,3 +1,4 @@ | |||
| @import './theme-default.less'; | |||
There was a problem hiding this comment.
Importing ./theme-default.less directly inside packages/abc/index.less breaks multi-theme support (such as dark, compact, and variable themes). When other themes (e.g., theme-dark.less or theme-variable.less) import @delon/abc/index.less, they will inadvertently pull in the default theme styles and variables, leading to style conflicts, overrides, and duplicate CSS rules. Component styles should remain theme-agnostic and rely on the global theme variables being present during compilation, rather than hardcoding a specific theme import.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2039 +/- ##
========================================
Coverage 95.14% 95.14%
========================================
Files 270 270
Lines 8776 8776
Branches 1811 1706 -105
========================================
Hits 8350 8350
Misses 338 338
Partials 88 88 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…-name imports Change all relative path imports (`../abc/`, `../chart/`, `../../../form/`) to package-name imports (`@delon/abc/`, `@delon/form/`) to fix pnpm strict module resolution. - Remove `@import '../chart/index.less'` from default.less/variable.less (chart is not a declared dependency of theme) - Add `@import './theme-default.less'` to abc/index.less for self-contained styles - Add `dist/` to generate-css.js paths (ordered before node_modules/) to resolve package-name imports during build - All 8 theme CSS files compile successfully with zero errors Close ng-alain/ng-alain#2631 Co-Authored-By: Claude <noreply@anthropic.com>
1877fcd to
316da32
Compare
After changing theme style imports from relative paths to package-name imports (@delon/abc/), the site:gen command failed because ng-alain-plugin-theme couldn't resolve @delon/abc/theme-variable.less from node_modules/ in the monorepo. Added 'ln -sf ../dist/@delon node_modules/@delon' to the site:gen script, creating a symlink that lets Less resolve @delon/abc/ imports through the standard node_modules/ search path. The dist/@delon/ directory is always built before site:gen in CI via build-delon.sh. Close ng-alain/ng-alain#2631 Co-Authored-By: Claude <noreply@anthropic.com>
…-name imports
Change all relative path imports (
../abc/,../chart/,../../../form/) to package-name imports (@delon/abc/,@delon/form/) to fix pnpm strict module resolution.@import '../chart/index.less'from default.less/variable.less (chart is not a declared dependency of theme)@import './theme-default.less'to abc/index.less for self-contained stylesdist/to generate-css.js paths to resolve package-name imports during buildClose ng-alain/ng-alain#2631
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information