meteor-scss
meteor-scss copied to clipboard
sass/node-sass -> sass/dart-sass
It's not so big of PR now! Works pretty well for me.
Side note: I'm also currently use the code snippet from https://github.com/Meteor-Community-Packages/meteor-scss/pull/258/commits/a4f6b9c54584613d8c7d3c558307d6e52eacb62b#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 : )
This PR is here in place of closed now https://github.com/Meteor-Community-Packages/meteor-scss/pull/295
It also works well with the code from https://github.com/Meteor-Community-Packages/meteor-scss/pull/258
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.
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. https://github.com/sass/dart-sass/issues/1100
Also note that dart-sass, while faster than the old ruby implementation is still substantially slower than libsass.
libsass and node-sass are officially deprecated now https://github.com/sass/node-sass/commit/e2391c259167a9692f2c23b0c91caa37502334ca
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
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.
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.
@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:
@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:
https://github.com/sass/dart-sass/issues/1138 https://github.com/sass/dart-sass/issues/1137
@sebakerckhof any updates on this? Really need to import sass from node_modules
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
Would that be possible to revisit this PR, since the blocker https://github.com/sass/dart-sass/issues/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 👍
Hi there, any ETA for merge?
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.
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!
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!
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.
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.
https://github.com/Meteor-Community-Packages/meteor-scss/issues/294
Sorry I think the confusion comes from my wording "they are deprecated now" in https://github.com/Meteor-Community-Packages/meteor-scss/issues/294#issuecomment-720125498 . What I meant there is that node-sass is deprecated.
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)
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