sass/node-sass -> sass/dart-sass

[dearlordylord]

It's not so big of PR now! Works pretty well for me.

Side note: I'm also currently use the code snippet from a4f6b9c#diff-2528c563afd9125ec438980adf8b65ca

      for(const possibleFile of possibleFiles){
        if (possibleFile.match(/\.css$/)) {
          let decodedPath = decodeFilePath(possibleFile, true);
          if (decodedPath && fileExists(decodedPath)) {
            return { absolute: true, path: decodedPath };
          }
        }
      }

to add css handling, would be nice to merge it sometime later : )

[dearlordylord]

This PR is here in place of closed now #295

It also works well with the code from #258

[znerol]

Sorry for the long delay and thanks a lot. Just tested this on a small project and it looks fine. Honestly I have no idea why the tests are failing (the failures are probably not related to this PR).

@sebakerckhof could you take a look at this? In my opinion it would be a good move to get away from node-sass since dart-sass is the official upstream nowadays.

[sebakerckhof]

This fails because dart-sass can't handle non-alpahetic characters at the start of an import, which we use for package imports, in line with other meteor css preprocessors. sass/dart-sass#1100

Also note that dart-sass, while faster than the old ruby implementation is still substantially slower than libsass.

[dearlordylord]

libsass and node-sass are officially deprecated now sass/node-sass@e2391c2

[sebakerckhof]

I had a discussion with the dart-sass people about the problem with the non-alphnumeric first character of the import. Long story short, they don't want to / can change it, so we would have to change it. This would be a major breaking change. So I'd like to schedule it together with Meteor 2.0

[sebakerckhof]

FYI: i'm working on a version with dart-sass. A bit more work is required than I thought at first sight and I hit a bug in dart-sass which is a blocker for now. But as soon as the bug is resolved I should have a beta pretty soon. As stated before, the import syntax is not going to be backwards compatible.

[moberegger]

Hey @sebakerckhof. Do you have a branch available that you could push? Not sure if I will be much use, but I could certainly try to help here.

[moberegger]

@sebakerckhof

Also note that dart-sass, while faster than the old ruby implementation is still substantially slower than libsass.

I saw this note in the README, just in case you weren't aware. No idea how much that will help, though.

Note however that by default, renderSync() is more than twice as fast as render() due to the overhead of asynchronous callbacks. To avoid this performance hit, render() can use the fibers package to call asynchronous importers from the synchronous code path. To enable this, pass the Fiber class to the fiber option:

[sebakerckhof]

@moberegger Yeah, and since we're using meteor, we can easily use fibers (but maybe not such a great idea seeing how fibers is deprecated). Anyway, performance issues aside, I'm still waiting on these 2 bugs to be fixed in dart-sass before we can have a working version based on dart-sass:

sass/dart-sass#1138
sass/dart-sass#1137

[kolyasya]

@sebakerckhof any updates on this? Really need to import sass from node_modules

[dearlordylord]

Thank you for looking into this! Really appreciate it so far.

I've updated PR to be compatible with current master version, and also bumped sass lib version (checked it as well, it works on my project well)

Looking forward for further news on this one!

I've noticed that bugs sass/dart-sass#1138 sass/dart-sass#1137 are resolved now, which is great news

[dearlordylord]

Would that be possible to revisit this PR, since the blocker sass/dart-sass#1137 was resolved? If dart-sass ends up being used in this package in some way or another, it'll increase its business value dramatically 👍

[goyan]

Hi there, any ETA for merge?

[StorytellerCZ]

Can people please take this for test. I currently don't have any scss project.
I'll merge this for a major release next month if there are no complains.

[faburem]

I have tested this in a decent code base and could not find any issues - both for the local and production build. I think it is a pretty representative project using bootstrap and a bunch of Sass features. While testing, I bumped the sass dependency version to 1.64.2 and did not see any problems either.
Since node-sass depends on a rather old Ruby version it is difficult to run this on new Apple silicon Macs (see https://forums.meteor.com/t/solved-fourseven-scss-package-problem-during-installation-node-sass-on-macos/58002/14), thus I think releasing this would help everyone on such a platform!

[dearlordylord]

I have tested this in a decent code base and could not find any issues - both for the local and production build. I think it is a pretty representative project using bootstrap and a bunch of Sass features. While testing, I bumped the sass dependency version to 1.64.2 and did not see any problems either. Since node-sass depends on a rather old Ruby version it is difficult to run this on new Apple silicon Macs (see https://forums.meteor.com/t/solved-fourseven-scss-package-problem-during-installation-node-sass-on-macos/58002/14), thus I think releasing this would help everyone on such a platform!

Thank you for looking into it @faburem . Updated version in PR to 1.64.2, hope it'll get merged soon!

[faburem]

LGTM

[StorytellerCZ]

Since it seems like even after updating to the latest node-sass the main issue which promted this PR won't be fixed I will release this as a major next version.

[dearlordylord]

Since it seems like even after updating to the latest node-sass the main issue which promted this PR won't be fixed I will release this as a major next version.

I think there could be some misunderstanding because this PR does fix the main issue which prompted this PR for me at least, and I successfully use it in my production code. Unless something changed.

#294

[dearlordylord]

Sorry I think the confusion comes from my wording "they are deprecated now" in #294 (comment) . What I meant there is that node-sass is deprecated.

[dsmalicsi]

Hi @Firfi, I tried testing this version on our project, but I'm getting these errors:

=> Errors prevented startup:                  

   While processing files with fourseven:scss (for target web.browser):
   /client/main.scss: Scss compiler error: Error: File to import: ./general not found in file: undefined
   ╷
   2 │ @import './general';
   │         ^^^^^^^^^^^
   ╵
   {}\imports\stylesheets\components.scss 2:9  @import
   {}\client\main.scss 7:9                     root stylesheet

   error: Scss compiler error: Error: File to import: ./general not found in file: undefined
   ╷
   2 │ @import './general';
   │         ^^^^^^^^^^^
   ╵
   {}\imports\stylesheets\components.scss 2:9  @import
   {}\client\main.scss 7:9                     root stylesheet


=> Your application has errors. Waiting for file change.

I tried changing it to just 'general' and even 'general.scss' './general.scss', but no luck.

I'm not sure what causes it (path issues, maybe?)

When I reverted to the latest version, it started working again.

Tested using Meteor 2.12, both on Windows 11 and Mac OS 13.0 (Intel)

[dearlordylord]

Hi @Firfi, I tried testing this version on our project, but I'm getting these errors:

=> Errors prevented startup:                  

   While processing files with fourseven:scss (for target web.browser):
   /client/main.scss: Scss compiler error: Error: File to import: ./general not found in file: undefined
   ╷
   2 │ @import './general';
   │         ^^^^^^^^^^^
   ╵
   {}\imports\stylesheets\components.scss 2:9  @import
   {}\client\main.scss 7:9                     root stylesheet

   error: Scss compiler error: Error: File to import: ./general not found in file: undefined
   ╷
   2 │ @import './general';
   │         ^^^^^^^^^^^
   ╵
   {}\imports\stylesheets\components.scss 2:9  @import
   {}\client\main.scss 7:9                     root stylesheet


=> Your application has errors. Waiting for file change.

I tried changing it to just 'general' and even 'general.scss' './general.scss', but no luck.

I'm not sure what causes it (path issues, maybe?)

When I reverted to the latest version, it started working again.

Tested using Meteor 2.12, both on Windows 11 and Mac OS 13.0 (Intel)

Is it possible for you to share the project or a minimal reproduction? I can try to look into it @dsmalicsi

[illusionfield]

Hi,

I've spent quite some time working on the integration of Meteor JS and Dart Sass. Given how things are evolving with Dart Sass, I wanted to share my approach before creating a separate pull request. I'm really curious to hear the feedback from the maintainer community on whether this direction is viable. I haven’t had the chance to test it comprehensively on different platforms like Windows, so I’d appreciate any input on that as well.

Here’s the repo where I’ve explored the idea: my meteor-scss. If the community finds it useful and worth developing further, I’d be more than happy to collaborate and get this merged into the main package. Also, I’d be interested in contributing to the ongoing maintenance and development of this package in the future.

Not too long ago, I created an issue about this topic (#322), but I realize now that this might not have been the right place to post it since it seems this PR already addresses some of the points I mentioned. :)

[StorytellerCZ]

@illusionfield I'm all for it. If you want to step up to maintain this package I'm happy to facilitate that.

[illusionfield]

Thanks, @StorytellerCZ,
I’d be really up for it! I’d be happy to step up as a maintainer for this package.
Let me know what the next steps are to make this official, and if there’s anything specific I should start with.
How should I present my version of the package? Would it make sense to connect directly with you to go over things, or is there another way you'd prefer?

I didn’t make a pull request yet because my approach brings a few new elements that are a bit different:

• It requires Meteor 2.7.3 as a minimum, but I’ve mainly optimized it for Meteor 3.0.
• Dropped the seba:minifiers-autoprefixer dependency and aligned it with Meteor’s standard-minifier, which now generates perfect mappings.
• Reworked the scss-config.json for improved handling.
• The includePaths config option seems less relevant now, but this might need further review to cover any cases I might have missed.
• Due to unique import strategies in both Meteor and Dart Sass, I had to sync up the import functionality, so it’s not fully backward-compatible (I’ve outlined this here: illusionfield/meteor-scss readme#importing). Dart Sass's importer returns a URL object, while its input is an URL string, which would have broken the @import "{my-package:pretty-buttons}/..."; implementation.

Let me know how you’d like to proceed – happy to jump in wherever it’s useful!

Cheers,
József

[illusionfield]

One of my ideas is to create a version of the plugin that acts purely as a bridge between the Sass compiler and Meteor. In this setup, developers could freely install the Dart Sass dependency via npm, depending on their preference (dart-sass or dart-sass-embedded). The plugin would detect and handle both, as each has a different compile method that offers better performance in specific cases.

This approach would make the Dart Sass dependency largely version-independent, requiring only compatibility checks between the plugin and specific Sass versions. Developers would have the flexibility to use whichever version of Sass they need, without relying on a specific version embedded in this package.

It would also significantly reduce the need for frequent version or patch updates for this package while maintaining flexibility for developers.

What do you think about this idea?