sass / node-sass

:rainbow: Node.js bindings to libsass
https://npmjs.org/package/node-sass
MIT License
8.51k stars 1.32k forks source link

Add _index.scss resolution when using @import on a directory #2339

Open mattpilott opened 6 years ago

mattpilott commented 6 years ago

I have hit a point where this would be really useful for myself and the team. Take a look at https://github.com/sass/sass/issues/690

Theres a fair bit of discussion resolving with the addition of this feature, I am wondering if this is on the radar and when it will be available.

Thanks!

nschonni commented 6 years ago

The support has landed in libsass with https://github.com/sass/libsass/commit/3af837c4e6053487186cfcf564a990ff11e4e356 but won't be available here till the next libsass tagged release is brought into node-sass

mattpilott commented 6 years ago

Any timescales on that @nschonni ?

oqx commented 6 years ago

Seconding @matt3224 here. Is there a timeline on this feature?

nschonni commented 6 years ago

Libsass was bumped in 4.9 https://github.com/sass/node-sass/commit/9d6faf6f0a005dc73870b1f87268014a82f170e3 so this should be available

nschonni commented 6 years ago

Actually doesn't look like it was https://github.com/sass/node-sass/blob/master/src/libsass/src/file.cpp

Chlebamaticon commented 6 years ago

It still doesn't work. :(

I've generated angular project with @angular/cli@6.0.8 and dependency that's responsible for compiling whole project(@angular-devkit/build-angular@~0.6.8) is using node-sass@4.9.1. As far as I know it's also compiling all the .sass files within application.

To the point, am receving such error after attempting to import file called '_index.scss' from directory 'src/styles'.

@import './styles';
^
      File to import not found or unreadable: ./styles.
      in /Users/<user>/Projects/<name>/src/index.scss (line 2, column 1)
moraveyo commented 6 years ago

Doesn't work for me too. ( And the doc says it's already there: https://sass-lang.com/documentation/file.SASS_REFERENCE.html#index_files

I invented this hack: Instead of index.scss or _index.scss I make a _.scss.

For exapmle block/_.scss

Then I import like that: @import "block/"; Everything works fine, although syntax check of my IDE is not very happy.

mattfelten commented 5 years ago

I'm looking for this too. Just tested the current version of node-sass to see if it's updated in 4.11, but it's not yet. https://github.com/mattfelten/node-sass-index-test

mattfelten commented 5 years ago

Just looked into this more. Looks like libsass has the change required in 3.6, but that hasn't been released yet. Looks like we're waiting for 3.6 to be released first.

https://github.com/sass/libsass/issues/2772#issuecomment-442716577 says it'll be released around Xmas, so converting from internet time, we'll have this feature by June.

PolyPik commented 5 years ago

libsass 3.6 got released just under a week ago.

How fast can we release a new version of node-sass that uses libsass 3.6?

xzyfer commented 5 years ago

It'll be a couple weeks..it's a complicated process and I'm off on vacation till the end of June.

On Fri., 24 May 2019, 4:34 am PolyPik, notifications@github.com wrote:

libsass 3.6 https://github.com/sass/libsass/releases/tag/3.6.0 got released just under a week ago.

How fast can we release a new version of node-sass that uses libsass 3.6?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sass/node-sass/issues/2339?email_source=notifications&email_token=AAENSWDKVVHQXRGXYBLPF43PW3PSNA5CNFSM4E3WV7XKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDDI6Q#issuecomment-495334522, or mute the thread https://github.com/notifications/unsubscribe-auth/AAENSWBDQETGJXL7DMTKUF3PW3PSNANCNFSM4E3WV7XA .

nottrobin commented 5 years ago

@mattfelten so converting from internet time, we'll have this feature by June.

June was a pretty good estimate (fingers crossed)

PolyPik commented 5 years ago

So is this change to libsass 3.6 going to be a minor release or a major release?

PolyPik commented 5 years ago

@xzyfer Do we have an ETA on this improvement?

fetis commented 5 years ago

so the documentation https://sass-lang.com/documentation/at-rules/import#index-files lies saying it's available in "LibSass since 3.5.0"?

I'm trying to make it work with node-sass v 4.12.0 which has LibSass 3.5.5 under the hood.

PolyPik commented 5 years ago

There seems to be a drought of commits and PRs.

dnemoga commented 5 years ago

Any updates?

cactysman commented 5 years ago

I would actually like to see this thing being configurable but with a fallback to _index. That way it doesn't have to be that name and also it won't break existing setups if people can just give it a new name.

nschonni commented 5 years ago

@metaa that would be a feature request to sass/sass to change the language

payneio commented 4 years ago

Still not in node-sass 4.13.0, either, even though the docs made me believe it would work.

saper commented 4 years ago

@payneio which docs?

mattpilott commented 4 years ago

Have you tried switching to dart sass. I use this now on all projects, has index feature. npm i sass

https://www.npmjs.com/package/sass

payneio commented 4 years ago

Ahh, @saper, now, I see https://sass-lang.com/documentation/at-rules/import#index-files indicates "LibSass since 3.6.0". Looking into what is packaged with node-sass, it looks like it is currently 3.5.0. I assumed if I downloaded the latest node-sass that it would work. Is there any way to get node-sass with LibSass 3.6.0?

lachieh commented 4 years ago

@xzyfer Indicated that it wouldn't be a quick upgrade. If someone else wants to attempt the upgrade, submit a PR.

nottrobin commented 4 years ago

@mattfelten so converting from internet time, we'll have this feature by June.

June was a pretty good estimate (fingers crossed)

Turns out June was a terrible estimate

kreo commented 4 years ago

you can use _.scss instead of index.scss like this @import "path/to/scss/files/" and it works

PolyPik commented 4 years ago

Is there documentation explaining how node-sass is built? I intend to upgrade the libsass version myself if @xzyfer will not do it.

I know it involves using node-gyp to build libsass and then there are bindings that make libsass work with NodeJS.

However a more or less official explanation would still help a lot.

saper commented 4 years ago

@polypik you can build node-sass with a pre-built libsass using LIBSASS_EXT, LIBSASS_CFLAGS and LIBSASS_LDFLAGS environment variables, see example at https://github.com/saper/ports-exp/blob/06f2c7fa24091d143d603c5c86e4b8e3c3982ecf/textproc/node-sass/Makefile#L27-L29

libsass_pic.a can be built by packing object files from the src/.libs directory of libsass into one file like in this line https://github.com/saper/ports-exp/blob/06f2c7fa24091d143d603c5c86e4b8e3c3982ecf/textproc/libsass35/Makefile#L33 This is not necessary if you can live with dynamic linking with the already installed libsass.so, in that case just point to it using LIBSASS_LDFLAGS.

Alternatively, you can update the source in the node-sass directory as checked out from git in src/libsass. Make sure you have that submodule installed. npm install from there should do the job.

Before you build the binding, please update package.json "libsass" value to indicate the version of the libsass you are using, possibly with some indicator this is your change.

Of course - any problems resulting from this remain your own...

PolyPik commented 4 years ago

src/libsass appears to be a subtree not a submodule. If it was a submodule, the Github website would show a different icon for that directory.

Also what branch should I make these changes to when I fork the repo? The master branch?

saper commented 4 years ago

@polypik currently v4.13.1 release tag and the master branch are almost identical as we speak,

PolyPik commented 4 years ago

There is also the v5 branch, which is only 4 commits behind master. Perhaps that branch should be rebased on the v4.13.1 tag and then have the libsass update be a commit on the v5 branch.

saper commented 4 years ago

Your call, @PolyPik

PolyPik commented 4 years ago

I have managed to merge the latest master branch changes into branch v5, updated the libsass subtree to 3.6.3 in v5, and successfully built the libsass library after adjusting the libsass.gyp file.

Some of the tests are obviously going to break and this is where I am at. The api.js tests were a relatively easy fix (there was just one issue and it was with the quote function. It no longer allows non-string input as according to recent specs). The number of broken spec.js tests are much more numerous and several tests seem to fail even though the expected and the actual values appear to be identical.

I tried upgrading the sass-spec version to 3.6.3 hoping to fix the issue, but then I realized that at some point in the past, the sass-spec repo did away with separate input and output files. Instead the input and output values are contained within one .hrx file. Which means if I were to change the version, the test file must also be changed to be able to properly read the spec files.

EDIT: Nvm about the expected and the actual values appearing identical. I just noticed some of the differences.

EDIT2: As it turns out the sass-spec repo didn't completely do away with separate input and output files. Some testcases still have separate input.scss and output.css. IMO this makes things even more difficult.

PolyPik commented 4 years ago

Which part of libsass is response for the error message of the following format? Error: Invalid UTF-8 character "\xE5"

One of the spec tests (libsass-closed-issues/issue_2446) is failing due to the error message being Error: Invalid UTF-8 sequence

I have grepped the source files in order to find where that message originates but it just doesn't seem to exist even in libsass 3.5.5.

EDIT: Nvm I don't that it is the fault of libsass.

saper commented 4 years ago

One of the spec tests (libsass-closed-issues/issue_2446) is failing due to the error message being Error: Invalid UTF-8 sequence

https://github.com/sass/libsass/blob/3.5.5/src/parser.cpp#L109

Can we move it to the v5 / upgrade libsass issue?

PolyPik commented 4 years ago

Nvm, I found the real cause of the failing issue_2446 test.

https://github.com/sass/sass-spec/commit/e6baa156

I adjusted the spec.js to account for this change in structure.

It fixed that test and issue_245443, but now other tests just broke as a result that change.

PolyPik commented 4 years ago

It.is.done.

2859

abirmingham commented 4 years ago

@PolyPik hooray! Does that mean that this will arrive soon? EDIT: Please interpret this as excitement rather than pressure :)

PolyPik commented 4 years ago

It.is.merged.

2859