Skip to content

fix(css): fix lightningcss dep url resolution with custom root#18125

Merged
sapphi-red merged 9 commits intovitejs:mainfrom
hi-ogawa:fix-lightningcss-dep-url-replace
Sep 26, 2024
Merged

fix(css): fix lightningcss dep url resolution with custom root#18125
sapphi-red merged 9 commits intovitejs:mainfrom
hi-ogawa:fix-lightningcss-dep-url-replace

Conversation

@hi-ogawa
Copy link
Copy Markdown
Contributor

@hi-ogawa hi-ogawa commented Sep 17, 2024

@bolt-new-by-stackblitz
Copy link
Copy Markdown

bolt-new-by-stackblitz Bot commented Sep 17, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@hi-ogawa hi-ogawa marked this pull request as ready for review September 17, 2024 04:25
@sapphi-red
Copy link
Copy Markdown
Member

sapphi-red commented Sep 17, 2024

I pushed a commit that renames root dir to dir dir because root dir has a special meaning and the test wasn't actually testing the issue.

vite/playground/vitestSetup.ts

Lines 138 to 140 in 993d45b

// when `root` dir is present, use it as vite's root
const testCustomRoot = path.resolve(testDir, 'root')
rootDir = fs.existsSync(testCustomRoot) ? testCustomRoot : testDir

@sapphi-red sapphi-red added feat: css p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release labels Sep 17, 2024
sapphi-red
sapphi-red previously approved these changes Sep 17, 2024
View reviewed changes
@sapphi-red
Copy link
Copy Markdown
Member

sapphi-red commented Sep 17, 2024

Sorry, I think I just mixed things up. The first code was correct. 😅

@hi-ogawa
Copy link
Copy Markdown
Contributor Author

hi-ogawa commented Sep 17, 2024

I pushed a commit that renames root dir to dir dir because root dir has a special meaning and the test wasn't actually testing the issue.

Aha, I didn't notice that. I initially used playground/css-lightningcss/root with a separate config vite.config-root.ts dceaaf3 , but things got broken, so I just moved it off to a separate playground css-lightningcss-root.

@sapphi-red sapphi-red merged commit eb08f60 into vitejs:main Sep 26, 2024
@hi-ogawa hi-ogawa deleted the fix-lightningcss-dep-url-replace branch September 30, 2024 05:24
sapphi-red pushed a commit that referenced this pull request Oct 14, 2024
@hi-ogawa @sapphi-red
fix(css): fix lightningcss dep url resolution with custom root (#18125)
eae00b5
moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025
@hi-ogawa @moonlitusun
fix(css): fix lightningcss dep url resolution with custom root (vitej…
c5b9d04 
…s#18125)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluwy bluwy bluwy approved these changes

@sapphi-red sapphi-red sapphi-red approved these changes

Assignees

No one assigned

Labels

feat: css p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Relative CSS asset import does not work with root config set

3 participants

@hi-ogawa @sapphi-red @bluwy