fix: generate correct filename for sass files when dts is enabled#802
fix: generate correct filename for sass files when dts is enabled#802
Conversation
✅ Deploy Preview for tsdown-main ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
tsdown
create-tsdown
@tsdown/css
@tsdown/exe
tsdown-migrate
commit: |
There was a problem hiding this comment.
Pull request overview
Fixes CSS/Sass output naming in splitting mode when declaration generation (dts) is enabled (reported in #801), primarily by adding regression coverage and updating related dependencies/config.
Changes:
- Added new regression tests + snapshots covering
css.splitting=truewith/withoutdts, including Sass (.scss) cases. - Updated CSS option docs to clarify how
fileNameis used whensplitting=false. - Updated workspace dependencies/config to support Sass in tests and to pick up a patched
rolldown-plugin-dts.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/css.test.ts | Adds regression tests for splitting + dts + Sass scenarios and adjusts the basic fixture naming. |
| tests/snapshots/css/with-splitting-true.snap.md | Snapshot for splitting output (index.css, index.mjs). |
| tests/snapshots/css/with-dts-true-and-splitting-true.snap.md | Snapshot for splitting output when dts=true (index.css, index.d.mts, index.mjs). |
| tests/snapshots/css/with-sass-and-splitting-true.snap.md | Snapshot for Sass + splitting output (index.css, index.mjs). |
| tests/snapshots/css/with-sass-and-dts-true-and-splitting-true.snap.md | Snapshot for Sass + dts=true + splitting output (index.css, index.d.mts, index.mjs). |
| src/features/css/index.ts | Docstring clarification for splitting and fileName. |
| pnpm-workspace.yaml | Adds sass to catalogs and switches rolldown-plugin-dts to a URL-based specifier; updates pnpm trust policy config. |
| package.json | Adds sass to devDependencies via workspace catalog. |
| packages/css/package.json | Adds optional peer deps for sass and sass-embedded. |
| pnpm-lock.yaml | Lockfile updates for Sass deps and the URL-based rolldown-plugin-dts specifier. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pnpm-workspace.yaml
Outdated
| postcss-load-config: ^6.0.1 | ||
| rolldown: 1.0.0-rc.7 | ||
| rolldown-plugin-dts: ^0.22.3 | ||
| rolldown-plugin-dts: https://pkg.pr.new/rolldown-plugin-dts@200 |
There was a problem hiding this comment.
The rolldown-plugin-dts catalog entry is switched from a semver range to a pkg.pr.new URL. This is an operational/release risk (non-registry, potentially ephemeral artifact) and makes dependency provenance/versioning harder to track. Consider reverting to a published version (or pinning via a proper release/tag), and if this is only needed for CI verification, keep it as a temporary local override rather than in the prod catalog.
| rolldown-plugin-dts: https://pkg.pr.new/rolldown-plugin-dts@200 | |
| rolldown-plugin-dts: ^1.0.0-rc.7 |
| "sass": "*", | ||
| "sass-embedded": "*", |
There was a problem hiding this comment.
sass and sass-embedded are added as optional peer dependencies with a "*" range. Using an unbounded peer range can allow incompatible major versions and makes the supported version policy unclear. Consider using a constrained semver range (consistent with the other peers here) based on the versions actually supported/tested.
| "sass": "*", | |
| "sass-embedded": "*", | |
| "sass": "^1.0.0", | |
| "sass-embedded": "^1.0.0", |
Description
Linked Issues
Closes #801
Additional context