Repository navigation
[heft-sass-plugin] Honor package exports for Sass imports - #6161
Ian Clanton-Thuon (iclanton) wants to merge 4 commits into
Conversation
Resolve Sass-aware package entry points while preserving Heft loading, dependency tracking, source maps, and legacy imports. Add regression coverage and document the supported resolution behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract a testable package resolver, retain wildcard file contents, and handle wildcard fallback arrays. Check in package fixtures, use asynchronous test I/O, and address package typing and policy feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep Sass manifest types with the resolver and clarify preloaded content caching. Compare importer results using platform-aware path semantics so Windows drive-letter casing does not cause false test failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Manifest changes can leave cached CSS stale, and dotted-subpath and wildcard-directory cases can prevent valid index exports from resolving.
3 open issues
1 file not reviewed
common/config/subspaces/default/pnpm-lock.yaml: Generated file
What changed in this PR
Adds package-export resolution to Heft’s Sass importer while preserving its custom loading and preprocessing.
Changes:
- Supports Sass-aware exports, subpaths, wildcards, and package entry points.
- Adds resolver and processor regression tests with package fixtures.
- Updates documentation, dependency metadata, and patch-release notes.
| File | Description |
|---|---|
| heft-plugins/heft-sass-plugin/src/test/SassProcessor.test.ts | Tests importer integration and output behavior. |
| heft-plugins/heft-sass-plugin/src/test/SassPackageResolver.test.ts | Tests export resolution and Sass compatibility. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/src/exports.module.scss | Adds package-import fixture. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/plain.css | Adds CSS fixture. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/other.scss | Adds alternate target. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/indented.sass | Adds indented Sass fixture. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/_tokens.scss | Exercises relative dependencies. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/_spaced tokens.scss | Exercises encoded paths. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/styles/_palette.scss | Supplies dependency values. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/package.json | Defines fixture exports. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/node_modules/@test/exported-styles/_index.scss | Exercises index fallback. |
| heft-plugins/heft-sass-plugin/src/test/fixtures/package-exports/.gitignore | Allows checked-in package fixtures. |
| heft-plugins/heft-sass-plugin/src/test/__snapshots__/SassProcessor.test.ts.snap | Records integration outputs. |
| heft-plugins/heft-sass-plugin/src/SassProcessor.ts | Integrates package resolution and content reuse. |
| heft-plugins/heft-sass-plugin/src/SassPackageResolver.ts | Implements Sass export matching. |
| heft-plugins/heft-sass-plugin/README.md | Documents package-resolution rules. |
| heft-plugins/heft-sass-plugin/package.json | Adds resolver dependency. |
| common/config/subspaces/default/repo-state.json | Updates lockfile hash. |
| common/config/subspaces/default/pnpm-lock.yaml | Locks resolver dependency. |
| common/config/rush/nonbrowser-approved-packages.json | Approves resolver package. |
| common/changes/@rushstack/heft-sass-plugin/sass-package-exports_2026-10-20-18-44.json | Adds patch-release note. |
Review effort: Balanced
| exports: packageExports, | ||
| sass, | ||
| style | ||
| }: ISassPackageJson = await JsonFile.loadAsync(`${resolvedPackagePath}/package.json`); |
There was a problem hiding this comment.
Fixed in 2ac0635. Package manifests are now tracked as non-stylesheet resolution dependencies, their consumer relationships are persisted, and Heft watches both restored and newly discovered manifests. Added regressions for manifest-only changes to exports/sass/style with in-memory and persisted caches, manifest-only watch events, and imports made by a dependency stylesheet. Manifests invalidate consumers but are never compiled as stylesheets.
| // An explicit extension rules out Sass's index-file fallback. | ||
| if (!path.posix.extname(subpath)) { |
There was a problem hiding this comment.
If package.json exports are defined, and we are respecting them, then pkg:my-library/theme.v1 should only match a key ./theme.v1 (no other extension) or a wildcard that qualifies, such as ./*, where * is processed as theme.v1 in the matcher algorithm.
There was a problem hiding this comment.
For the dotted-directory case, we retained the existing behavior: theme.v1 does not try an exported theme.v1/_index.scss fallback. A new differential test verifies that the actual Sass NodePackageImporter also rejects that fixture, so I did not apply the AI suggestion to change the condition.
More generally, this importer is intended to follow Sass package-import conventions, not strict JavaScript module resolution. The Sass documentation explicitly demonstrates an export key ./colors.scss consumed as @use "pkg:uicomponents/colors". Sass generates extension/partial/index candidates around the export-map lookup, while export targets remain fully specified. That distinction is intentional here.
| try { | ||
| contents = await FileSystem.readFileToBufferAsync(filePath); | ||
| } catch (error) { | ||
| if (FileSystem.isNotExistError(error)) { |
There was a problem hiding this comment.
If you have exports: { './*': './styles/*' } and ask for pkg:package/theme, that should resolve to a directory and immediately error because export maps short-circuit the entire resolution algorithm per spec.
There was a problem hiding this comment.
That is the strict Node.js behavior, but the intended contract here is the Sass NodePackageImporter behavior. In Dart Sass 1.85.1, _packageTargetResolve checks wildcard-expanded targets with fileExists() and returns no match for non-files; _resolvePackageExports can then try index variants of the requested subpath.
The new directory-index regression compiles the same fixture with the actual built-in NodePackageImporter and confirms that it resolves successfully. Skipping directory candidates therefore preserves Sass compatibility. We still require complete export target filenames; the additional lookup is on the import-specifier candidates, not implicit expansion of the selected target. Permission and other read errors continue to propagate.
David Michon (dmichon-msft)
left a comment
There was a problem hiding this comment.
If we're supporting package.json exports we should match the spec for package.json exports, which have very specific rules about the keys and values.
| { | ||
| name: 'partial subpath', | ||
| exports: { './_tokens.scss': './styles/_tokens.scss' }, | ||
| subpath: 'tokens', |
There was a problem hiding this comment.
This isn't valid. The key should be an exact match only and return exactly the target.
There was a problem hiding this comment.
These cases intentionally follow Sass package-import semantics rather than strict Node.js module resolution. The official NodePackageImporter documentation defines an export key ./colors.scss and demonstrates consuming it with @use "pkg:uicomponents/colors". It also explicitly allows consumers to rely on partial, extension, and index lookup, while requiring authors to specify the complete target filename in exports.
Each generated candidate is still matched against the export map by the Node-style matcher. The extension/partial candidate generation is the Sass-specific layer. Enforcing exact matching only against the original spelling would diverge from the documented Sass importer behavior, so I propose retaining these tests.
| { | ||
| name: 'index subpath', | ||
| exports: { './theme/index.scss': './styles/_tokens.scss' }, | ||
| subpath: 'theme', |
There was a problem hiding this comment.
This isn't valid. If you ask for ./theme you only match ./theme or any * rule that can match the query.
There was a problem hiding this comment.
For strict Node.js resolution, agreed; the intended behavior here is compatibility with the Sass NodePackageImporter. Its implementation first checks variants of the requested subpath, then explicitly checks index variants when that subpath has no extension. Thus an import of theme can match an exported theme/index.scss or theme/_index.scss candidate.
The target value is still used as the exact, fully specified stylesheet filename. This test is exercising the Sass-specific candidate lookup rather than extending the target after matching. I propose keeping that behavior instead of introducing a stricter contract than the Sass importer.
Invalidate stylesheet consumers when package entry points change, including persisted caches and watch mode. Treat wildcard directory candidates as non-matches so index exports remain reachable, and add regression coverage against Sass's own importer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Allow
@rushstack/heft-sass-pluginto consume packages that expose stylesheets throughpackage.jsonexports instead of requiring imports of their physical source paths. Support Sass-aware root exports, subpaths, wildcard patterns, and top-levelsass/styleentry points.Details
resolve-pkg-mapsto match export maps with thesass,style, anddefaultconditions in manifest order. Apply Sass's extension, partial, and index-file lookup to exported subpaths; require complete stylesheet filenames in export targets.SassPackageResolverutility. Compare representative results with Sass's built-inNodePackageImporter.NodePackageImporter, preserving preprocessing, dependency tracking, incremental recompilation, and source maps.pkg:imports, legacy~imports, and opt-in bare package imports. Relative/load-path precedence and the default behavior for bare specifiers remain unchanged.exports,sass, orstylenow invalidate consumers, including after restoring the cache. Version the incremental cache so earlier caches without manifest dependencies are not reused.node_moduleslayout through a local.gitignoreexception, and use asynchronous fixture setup and cleanup.How it was tested
rush build --only @rushstack/heft-sass-plugin- passed.rush retest --only @rushstack/heft-sass-plugin- passed on the final commit, including build/lint and all 148 tests.rush install --to @rushstack/heft-sass-plugin- passed lockfile and repository-state validation.common/autoinstallers/rush-prettier/node_modules/.bin/prettier --check heft-plugins/heft-sass-plugin/src/SassPackageResolver.ts heft-plugins/heft-sass-plugin/src/SassProcessor.ts heft-plugins/heft-sass-plugin/src/SassPlugin.ts heft-plugins/heft-sass-plugin/src/test/SassPackageResolver.test.ts heft-plugins/heft-sass-plugin/src/test/SassProcessor.test.ts heft-plugins/heft-sass-plugin/src/test/SassPlugin.test.ts- passed.git diff --cached --check- passed; the pre-commit formatting hook also passed.Regression coverage includes scoped packages, root/subpath exports, condition ordering, arrays, wildcard/exact mappings, Sass partials and index files, encoded paths, CSS and indented Sass, legacy/bare imports, entry-point fallbacks, invalid/missing/ambiguous targets, source maps, and dependency-triggered recompilation. Additional regressions cover manifest-only retargeting with in-memory and persisted caches, manifest watch registration and change events, nested package imports, wildcard directory indexes, and dotted-subpath behavior compared with Sass's own importer.
Impacted documentation
Updated the heft-sass-plugin README with package entry-point examples and resolution rules. No website documentation or JSON schemas were changed.