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

LibSass should not import CSS files, let alone as SCSS #2611

Closed
nex3 opened this Issue Mar 23, 2018 · 14 comments

Comments

Projects
None yet
8 participants
@nex3
Contributor

nex3 commented Mar 23, 2018

If I have two files:

// file1.scss
@import "file2";
// file2.css
$var: 12;
a {b: $var}

...LibSass will import "file2" and interpret it as SCSS rather than plain CSS. Neither of these behaviors are correct. .css files cannot be imported by Sass files without an importer, and if they could they should be interpreted as plain CSS without allowing any Sass features.

I recommend adding a deprecation warning for this as soon as possible, possibly along with a link to an example importer that will import .css files for users for whom migrating will be difficult.

@nex3 nex3 referenced this issue Mar 23, 2018

Closed

dart-sass? #435

@xzyfer

This comment has been minimized.

Contributor

xzyfer commented Mar 24, 2018

This has been a long running issue, and one of the biggest node sass pain points. Thean was to removed this feature/bug with the module system landed to reduce disruption.

See #754
See #1656
See #1963

@nex3

This comment has been minimized.

Contributor

nex3 commented Mar 24, 2018

I'd like to at least add a warning for this as soon as possible. Dart Sass will not be implementing this behavior, so I'd like to strongly warn people away from it.

@xzyfer

This comment has been minimized.

Contributor

xzyfer commented Mar 24, 2018

PR to add a deprecation warning #2613

@xzyfer xzyfer closed this in #2615 Mar 28, 2018

xzyfer added a commit to sass/node-sass that referenced this issue Apr 16, 2018

Don't watch or compile .css files
This was a non-standared feature in LibSass we had to support. It
is being removed in LibSass 3.6.0.

This prevents people from output their source directory.

See sass/libsass#2611
See #2184
See #2006
See #1933
See #1925
See #1867
See #1845
@tony

This comment has been minimized.

Contributor

tony commented Apr 28, 2018

I recommend adding a deprecation warning for this as soon as possible, possibly along with a link to an example importer that will import .css files for users for whom migrating will be difficult.

Please provide some examples of how to handle importing raw CSS files from third party libraries. This would break a lot of behavior for me. It was an important feature.

@xzyfer

This comment has been minimized.

Contributor

xzyfer commented Apr 28, 2018

@liamkeily

This comment has been minimized.

liamkeily commented Apr 30, 2018

Like @tony I was using this to import css files from an npm package, whats the best way to go about this now?

@xzyfer

This comment has been minimized.

Contributor

xzyfer commented Apr 30, 2018

See my previous comment

@bayucandra

This comment has been minimized.

bayucandra commented May 23, 2018

@xzyfer just check at documentation about "Custom importer": https://sass-lang.com/documentation/file.SASS_REFERENCE.html#custom_importers . However, it looks not so clear to me. Do you have more detail reference about "Custom import" things? thanks in advance.

@qodesmith

This comment has been minimized.

qodesmith commented May 31, 2018

@bayucandra If you're using Webpack, you can import your css files at the top of your entry JavaScript file. That's where I was importing my top-level scss file anyway. And so long as you're using css-loader in your chain of loaders, everything should be good to go.

@tcchau

This comment has been minimized.

tcchau commented May 31, 2018

@qodesmith I'm using create-react-app, which restricts you from importing from outside of the src directory, so this isn't a viable workaround for people like @liamkeily and myself. I work mainly with SASS so the times I want to import a CSS file are exactly when it's from a third-party who provides a CSS file but not a SASS file. Disallowing imports of CSS files altogether seems overly-restrictive, since SASS is a superset of CSS and a SASS parser should be able to handle all CSS input without problems.

@qodesmith

This comment has been minimized.

qodesmith commented May 31, 2018

@tcchau Shameless plug - try Create New App instead 😄I created it to ease my pain of including things like scss and redux since I was using them all the time. But I wanted to keep the simplicity of CRA in that you just have to type a single command and boom - app generated.

I agree that it's a pain that you can't import css directly from a scss file. I myself have been using it that way to import 3rd party libraries (even some of my own!) that only provide a css file.

@chriseppstein

This comment has been minimized.

Member

chriseppstein commented May 31, 2018

Disallowing imports of CSS files altogether seems overly-restrictive, since SASS is a superset of CSS and a SASS parser should be able to handle all CSS input without problems.

@tcchau We agree. The superset feature of Sass is exactly why this is a problem. The early decision to change the behavior of a CSS feature (@import) broke the superset guarantee by creating an alternate implementation. Right now, there's legal CSS that works differently when used with Sass than when used as CSS. Even if that's what you want when you use Sass, it violates the superset contract.

@qodesmith

This comment has been minimized.

qodesmith commented May 31, 2018

Right now, there's legal CSS that works differently when used with Sass than when used as CSS.

@chriseppstein I'd be interested to see an example of that. I wasn't aware of it!

@bayucandra

This comment has been minimized.

bayucandra commented Jun 1, 2018

@qodesmith Looks the point is just throw away CSS out from SCSS code right? I thought there was some way which I can do inside my SCSS code, so we can still see a CSS list inside SCSS code. Will do gulp concatenation then, since I am using gulp instead Webpack. Thanks for sharing your way.

nex3 added a commit to nex3/sass-loader that referenced this issue Jun 2, 2018

Stop testing CSS imports
This behavior is deprecated in LibSass and is not supported in Dart
Sass. See sass/libsass#2611.

nschonni added a commit to nschonni/node-sass that referenced this issue Jul 9, 2018

Don't watch or compile .css files
This was a non-standared feature in LibSass we had to support. It
is being removed in LibSass 3.6.0.

This prevents people from output their source directory.

See sass/libsass#2611
See sass#2184
See sass#2006
See sass#1933
See sass#1925
See sass#1867
See sass#1845

xzyfer added a commit to sass/node-sass that referenced this issue Jul 16, 2018

Don't watch or compile .css files
This was a non-standared feature in LibSass we had to support. It
is being removed in LibSass 3.6.0.

This prevents people from output their source directory.

See sass/libsass#2611
See #2184
See #2006
See #1933
See #1925
See #1867
See #1845

xzyfer added a commit to sass/node-sass that referenced this issue Jul 16, 2018

Don't watch or compile .css files
This was a non-standared feature in LibSass we had to support. It
is being removed in LibSass 3.6.0.

This prevents people from output their source directory.

See sass/libsass#2611
See #2184
See #2006
See #1933
See #1925
See #1867
See #1845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment