[Package Importer] Add types and specs

[jamesnw]

Continuation of #3660

Issue

Blocked until proposal is accepted.

• [Package Importer] Dart Implementation dart-sass#2130
• [Package Importer] Embedded Host embedded-host-node#260
• [Package Importer] Specs sass-spec#1952

[nex3]

Since baseURL is now a directory rather than a file path, and the Node.js algorithms expect a file path, we should explicitly add a basename before passing it into those algorithms.

[jamesnw]

Since baseURL is now a directory rather than a file path, and the Node.js algorithms expect a file path, we should explicitly add a basename before passing it into those algorithms.

@nex3 Based on my reading of the algorithm, the parentURL
doesn't make any assertions on whether it is a file or directory. Passing a file in a directory is identical to passing in the directory, as parentURL is only used for relative resolution.

It's also unclear to me what we would use as a basename when the user only provides a directory.

[ntkme]

I think what @nex3 means is that we can should pass /path/to/dir as /path/to/dir/.. Many times algorithm does not check if a path is a file or a directory, and this can potentially be dangerous. E.g. dirname('/path/to/dir') is different from dirname('/path/to/dir/.'), so that if an algorithm expect a file, it might be better to pass /path/to/dir/. to met the expectation.

[jamesnw]

I think what @nex3 means is that we can should pass /path/to/dir as /path/to/dir/.. Many times algorithm does not check if a path is a file or a directory, and this can potentially be dangerous. E.g. dirname('/path/to/dir') is different from dirname('/path/to/dir/.'), so that if an algorithm expect a file, it might be better to pass /path/to/dir/. to met the expectation.

@nex3 Does this match your expectations- adding . as a basename, and updating the spec to read-

  * Let `pkgImporter` be a [Node Package Importer] with an associated
    `entryPointURL` of the absolute file URL for `entryPointDirectory`
    appended with a [single-dot URL path segment](https://url.spec.whatwg.org/#single-dot-path-segment).

My reading of this is that this is only a change to the spec, and we don't need to update the implementation as well, correct?

[nex3]

The parentURL parameter that's passed into PACKAGE_RESOLVE is ultimately used in two places:

1. In PACKAGE_RESOLVE itself, node_modules/${packageSpecifier} is resolved relative to parentURL and then parentURL is set to dirname(parentURL).

2. It's passed to LOOKUP_PACKAGE_SCOPE, which immediately sets it to its own directory name at the beginning of the loop.

None of these operations actually looks at the basename. Even for resolving the relative path, node_modules/foo relative to a/b/c is a/b/node_modules/foo—the fact that the basename was c doesn't matter. This means, as @ntkme points out, that you need to pass in something like a/b/c/. if you want to actually check within a/b/c. I don't know why the Node.js algorithm is written this way... it seems silly to me to pass in information that's just going to be thrown out, which is why I wanted to avoid doing the same thing in the Sass API.

All that said, I think the best place to compensate for a weird API like this is immediately before invoking it. Store the information "at rest" in a way that makes the most sense semantically—as the directory that we first check for node_modules—and only add /. immediately before invoking the Node.js algorithm.

My reading of this is that this is only a change to the spec, and we don't need to update the implementation as well, correct?

That's right. The implementation now avoids the issue entirely by always operating in terms of directories in the first place.

[jamesnw]

@nex3 Good point about appending /. immediately before invoking the algorithm. I added it in e2517ca.