[node-sass incompatibility] Importers that return both file and contents keys. #975
Assignees
Labels
Comments
chriseppstein
added a commit
to linkedin/eyeglass
that referenced
this issue
Mar 29, 2020
* Upgrade to typescript 3.8 so that we can use `import type` statements. * Removes `node-sass` as a direct dependency. Eyeglass will attempt to require both `node-sass` and `sass` unless a sass engine is explicitly passed as an option. In the case where both are installed, `node-sass` is used. * All import statements to `node-sass` are now `import type` statements which ensures we won't accidently import a concrete aspect of the library into our module's namespace. * Testing infrastructure and updates to run tests against both implementations of Sass. * Ameliorations for incompatibilities between node-sass and dart-sass. * `dart-sass` has different behavior for importers that return both `file` and `contents`. A bug has been filed at sass/dart-sass#975. Note: Custom importers provided to eyeglass from eyeglass addons or applications using eyeglass are insulated from this incompatibility. * `dart-sass` doesn't support functional invocation of sass value types, so a lot of the code changes were to instantiate those classes with `new` instead. (I discussed this with @nex3 a while ago and she indicated that it wasn't an API difference that she intended to support.) Note: Eyeglass addons that wish to support dart-sass will also need to make this change. * `dart-sass` doesn't support the constants `sass.NULL`, `sass.TRUE` and `sass.FALSE`. Instead, the code in eyeglass has been updated to use the constants that are supported by both: `sass.types.Null.NULL`, `sass.types.Boolean.TRUE`, and `sass.types.Boolean.FALSE` respectively. An issue has been filed: sass/dart-sass#977 * `dart-sass` doesn't support the `nested` output style which was the default for node-sass. So all test output has been converted to expect the `"expanded"` output style.
Awjin
added a commit
that referenced
this issue
Apr 25, 2020
Instead, use `contents` with `file` as the canonical url. Closes #975.
Awjin
added a commit
that referenced
this issue
Apr 25, 2020
Instead, use `contents` with `file` as the canonical url. Closes #975.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In
node-sassif bothcontentsandfileare returned, the presence ofcontentswins and the file is not attempted to be read, instead the file name provided is used as that file's canonical url.In
dart-sassthe presence offilecauses the importer result to attempt to read the file from disk even if the contents are already present.The relevant code in dart-sass is here:
https://github.com/sass/dart-sass/blob/master/lib/src/importer/node/implementation.dart#L192
The behavior of
node-sassis preferable here because it allows importers to return the correct path to the file while also having already processed the results. For instance, the importer might do some preprocessing on the file to get it ready for parsing by Sass, but also wants to return the full url to the resolved location for error reporting purposes.The text was updated successfully, but these errors were encountered: