Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add proposal for pkg: dependency imports #3

Closed
wants to merge 43 commits into from
Closed
Changes from 4 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
cae27d5
Add proposal for pkg: dependency imports
jamesnw Jun 30, 2023
a763eed
Remove broken link
jamesnw Jun 30, 2023
9e8ab52
wrap
jgerigmeyer Jun 30, 2023
27523a2
fix link
jgerigmeyer Jun 30, 2023
14b5df6
Package importer 2nd draft
jamesnw Jul 11, 2023
b71dba5
Merge branch 'main' of github.com:oddbird/sass into package-loads
jamesnw Jul 12, 2023
38abde0
Merge branch 'main' into package-loads
jgerigmeyer Jul 13, 2023
2781c92
review
jgerigmeyer Jul 13, 2023
856e4c7
Clarify Vite comment
jamesnw Jul 13, 2023
4bfa2fb
review response
jamesnw Jul 14, 2023
7b30f49
Move type into package-importer
jamesnw Jul 17, 2023
358123a
Include md.d.ts files in toc scripts
jamesnw Jul 17, 2023
3c3af5e
Review response
jamesnw Jul 21, 2023
fd9fbd2
Remove Dart semantics
jamesnw Jul 21, 2023
54e4039
Add additional community examples
jamesnw Jul 21, 2023
c565c93
Merge branch 'main' of github.com:oddbird/sass into package-loads
jamesnw Jul 21, 2023
dadaa1d
Remove proposed option from options.d.ts
jamesnw Jul 21, 2023
9d549ee
Replace options with correct copy
jamesnw Jul 21, 2023
7cc0f47
Edits
jamesnw Jul 21, 2023
82b3739
Add procedures section
jamesnw Jul 21, 2023
a2a890d
Break out pkg url resolution, and define Package importer results
jamesnw Jul 21, 2023
2554f70
Document exports resolution, make vs more consistent
jamesnw Jul 21, 2023
6e10182
Add missing links
jerivas Jul 21, 2023
f8b6f1c
Add import to ts to satisfy ambient module error
jamesnw Jul 22, 2023
64a724e
Standardize language
jamesnw Jul 24, 2023
a4da3ac
Temporarily add debug info to link check
jamesnw Jul 24, 2023
031fec9
Revert debugging in link check
jamesnw Jul 24, 2023
d42ee46
Merge branch 'main' of github.com:oddbird/sass into package-loads
jamesnw Aug 8, 2023
daee9d9
Address review and style
jamesnw Aug 8, 2023
5a026bb
Allow style at root to load sass and scss files. Fix JS option name.
jamesnw Aug 8, 2023
949e1b3
Importer clarifications
jamesnw Aug 8, 2023
85ebde3
Sort procedures by appearance
jamesnw Aug 8, 2023
5dbe6dc
Updates to resolving package root values
jamesnw Aug 8, 2023
eff344d
Clean up root directory resolution
jamesnw Aug 8, 2023
676c409
Point Resolving package exports procedure to Node procedure
jamesnw Aug 9, 2023
7422ab8
Point package root procedure to Node procedures
jamesnw Aug 9, 2023
eb26657
Give a name to input url
jamesnw Aug 11, 2023
2adad7c
Lint
jamesnw Aug 15, 2023
5ba76f7
Merge branch 'main' of github.com:oddbird/sass into package-loads
jamesnw Aug 18, 2023
be55d14
Switch to pkgImporter: string option
jamesnw Aug 18, 2023
6c3025b
Add pkg importer support to legacy API
jamesnw Aug 18, 2023
b99d195
Add partial, extension and index matching on conditional export alias
jamesnw Aug 18, 2023
e926efa
Fix typo
jamesnw Aug 18, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 191 additions & 0 deletions proposal/package-importer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
# Package Importer

*([Issue](https://github.com/sass/sass/issues/2739))*

This proposal adds Sass support for a standard package importer, introducing a
`pkg` URL scheme to indicate Sass package imports in an implementation-agnostic
format.

## Table of contents

* [Background](#background)
* [Summary](#summary)
* [Design Decisions](#design-decisions)
* [Semantics](#semantics)
* [Deprecation Process](#deprecation-process)

## Background

> This section is non-normative.

Historically, Sass has not specified a standard method for using packages from
dependencies. A number of domain-specific solutions exist using custom importers
or by specifying a load path. This can lead to Sass code being written in a way
that is tied to a specific domain and make it difficult to rely on dependencies.

## Summary

> This section is non-normative.

Sass users often need to use styles from a dependency to customize an existing
theme or access styling utilities.

This proposal defines a `pkg` URL scheme for usage with `@use` that directs an
implementation to resolve a URL within a dependency. The implementation will
resolve the dependency URL using the standard resolution for that environment.
Once resolved, this URL will be loaded in the same way as any other `file:` URL.

For example, `@use 'pkg:bootstrap';` would be able to resolve to the path of a
library-defined export within the `bootstrap` dependency. In Node, that would be
resolved within `node_modules`, using the [node resolution algorithm]. In Dart,
that would be resolved within `pub-cache`, using [package-config].

[node resolution algorithm]: https://nodejs.org/api/packages.html
[package-config]: https://pub.dev/packages/package_config

There will be tension between the different path resolution algorithms that are
used. To address this, a `pkg:` url will only use the environment's resolution
initially in order to resolve a starting path. After that, it will be handed off
to the Sass algorithm, which works directly with the filesystem, to resolve
index files and partials.

To better understand and allow for testing against the recommended algorithm, a
[Sass pkg: test] repository has been made with a rudimentary implementation of
the algorithm.

[Sass pkg: test]: https://github.com/oddbird/sass-pkg-test

### Design Decisions

We could use the `~` popularized by Webpack's `load-sass` format, but this has
been deprecated since 2021. In addition, since this creates a URL that is
syntactically a relative URL, it does not make it clear to the implementation or
the reader where to find the file.

While the Dart Sass implementation allows for the use of the `package:` url
scheme, a similar standard doesn't exist in Node. We could add `node_modules` to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other major issue with node_modules is that a single file may have access multiple node_modules directories, and what's more different files may have access to different node_modules directories within the same compilation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 14b5df6

the load path, but that again suffers from lack of clarity to the implementation
and the reader.

Some npm packages have adopted a convention of declaring their Sass entrypoints
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a logical separation between design concerns for the platform-agnostic pkg: syntax and design concerns for the Node.js-specific package importer. Consider adding a heading here to visually separate out the latter.

using `"style"` or `"sass"` keys in their `package.json`. This is tied to the
npm context, and would require parsing package.json to find these files.
Instead, each implementation should use the standard patterns and tooling
(`require` for Node and `package_config` for Dart) for resolving file paths.
This allows library authors control over what is public, which follows Sass's
general design.

This proposal does not add support for a [Node custom user condition]. Node
currently does not provide a native way to set a condition on a single
invocation of `require.resolve`, meaning we would need to implement our own
`package.json` parsing similar to [Rollup] or add a dependency on a community
solution like [resolve.exports]. However, this proposal will not prevent future
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not intrinsically opposed to using something community-standard like resolve.exports, especially since it's so small.

support.

[Node custom user condition]: https://nodejs.org/api/packages.html#community-conditions-definitions
[Rollup]: https://github.com/rollup/plugins/blob/master/packages/node-resolve/src/package/resolvePackageExports.js
[resolve.exports]: https://github.com/lukeed/resolve.exports

A potential source of confusion is in instances where a dependency defines a
default or main export that is not in the root folder. In these cases, Sass will
look for index and partial files in the same folder as the default or main
export, and not in the package root.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this refer to a default or main JS export?

Can you document the reasoning behind this design decision as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be for any file defined as main or module, which for most packages would be a JS export. However, some packages do have other file extensions- see https://github.com/Decathlon/vitamin-web/blob/main/packages/sources/css/package.json#L21 for css. Bulma was the only instance of a sass main export I could find.

However, based on our conversation and discovery, I've reversed this and Sass will look relative to the package root instead of relative to the package main export.


The `pkg` scheme will not be supported in the browser version of Dart Sass. To
jamesnw marked this conversation as resolved.
Show resolved Hide resolved
support a similar functionality, a user would need to ensure that files are
served, and the loader would need to fetch the URL. In order to follow the same
algorithm for [resolving a file: URL], we would need to make many fetches. If we
instead require the browser version to have a fully resolved URL, we negate many
of this spec's benefits.
jamesnw marked this conversation as resolved.
Show resolved Hide resolved

[resolving a file: URL]: ../spec/modules.md#resolving-a-file-url

## Semantics

This proposal defines a new step in the [Loading a Source File] in modules that
implementations must include.

New step for [Loading a Source File], after "if argument is a relative URL"
step:

- If `argument` is a valid URL with scheme `pkg`:
- Let `resolved` be the result of resolving a pkg url
- If `resolved` is not null, let `argument` equal `resolved`, and continue.
Otherwise, return null.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two ways we could go here for defining "package importers":

  1. Reserve the URL scheme pkg: and dispatch it specially to a designated package importer, which is what this proposal does currently.
  2. Allow any importer to resolve pkg: imports and just provide an official recommendation for the structure of pkg: URLs as well as a built-in importer for Node.js semantics.

The benefits of option 1 are that you can give package importers some assistance in resolving URLs (as you do below), and that you can enforce more stringent requirements on them (like only processing pkg: URLs) than you can on general importers.

On the other hand, option 2 gives authors a lot more flexibility. For example, you can imagine someone wanting to support pkg: URLs in the browser via import maps, or even just from a virtual filesystem in web IDE. And authors who do want to target the filesystem can mitigate the pain by writing FileImporters that handle the pkg: URL scheme.

So my recommendation is this: Don't define any new language semantics. Instead, define the expectations for an importer that handles pkg: URLs: that the first component will be the package name, that it will automatically try to find an entrypoint file, that nested files can be addressed using /, and so on.


[Loading a Source File]: ../spec/modules.md#loading-a-source-file

### Resolving a `pkg`: URL

This algorithm takes a URL, `url`, whose `scheme` must be `pkg` and returns
either another URL of a file or directory on disk, or null. At this point, the
URL is not guaranteed to resolve to a file or disk.

- Let `resolved` be the result of the implementation's dependency resolution
algorithm.
- If resolution fails:
- Let `dependencyDefault` be the result of the implementation's dependency
resolution algorithm for the dependency name.
- Let `urlSubpath` be the path segments of `url` that are not part of the
dependency name.
- Let `resolved` be `dependencyDefault` appended with `urlSubpath`.
- If `resolved` is unknown, meaning neither the full path nor the dependency
name were resolved, return `null`.
- If `resolved` is a directory or a file with extension of `scss`, `sass`, or
`css`, return it. Otherwise:
- If `resolved` is a file with a name that is different than the name of `url`,
return the file's containing directory.
- Return null.

Note: The fifth step, returning the containing directory, is required to support
instances where a dependency's default export is not a `sass`, `scss` or `css`
file.

[Loading a Module]: ../spec/modules.md#loading-a-source-file

## Deprecation Process

The `package` url scheme supported in Dart is not part of the Sass spec, but is
supported by the `dart-sass` implementation when running in Dart. This should be
deprecated in favor of moving authors to the new `pkg` url scheme. Usage of the
`package` syntax will result in a deprecation message, and a future major
version of Sass should remove support.
jamesnw marked this conversation as resolved.
Show resolved Hide resolved

## Examples
jamesnw marked this conversation as resolved.
Show resolved Hide resolved

### Node
jamesnw marked this conversation as resolved.
Show resolved Hide resolved

For Bootstrap to support `pkg` imports, they have 2 options. They can add
`_index.scss` to `dist/js`, as that is the containing folder for their main
export, and use `@forward` to expose the files.

```
@forward './scss/bootstrap.scss';
```

Alternatively, they could recommend using `@use 'pkg:bootstrap/scss';`, and add
an `exports` key to their `package.json`, using node subpath exports. Opting
into this format requires library authors to be exhaustive. Here, they are
exposing the default js library, forwarding `./scss` and also exposing
`./scss/bootstrap.scss` to not break existing imports of `@use
'bootstrap/scss/bootstrap.scss'`.

```
"exports": {
".": "./dist/js/bootstrap.js",
"./scss": "./scss/bootstrap.scss",
"./scss/bootstrap.scss": "./scss/bootstrap.scss"
}
```

### Dart

For Dart libraries to take advantage of this, they can place a file at
`lib/_index.scss`. Other files within the `lib` folder would also be available
for library consumers. For example, if a library `libraryName` contains
`lib/themes/dark/_index.scss`, a consumer could write `@use
'pkg:libraryName/themes/dark'`.

More examples can be found in the [Sass pkg: test] example repo.

[Sass pkg: test]: https://github.com/oddbird/sass-pkg-test