Skip to content

Change order of node-sass, sass import #163

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

Closed
dummdidumm opened this issue Jun 5, 2020 · 5 comments
Closed

Change order of node-sass, sass import #163

dummdidumm opened this issue Jun 5, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request next-major will be fixed in the next major

Comments

@dummdidumm
Copy link
Member

Is your feature request related to a problem? Please describe.
Over at the language-tools repository we sometimes get error reports about using SCSS gives errors in the IDE. Most of the time it's because the users have node-sass installed and that is very sensitive to node versions. So if the IDE runs on a different node version than the user, it will likely crash if he does not set some configs, and even then it does not seem to work always. One suggestion we give the users is to use sass instead. Now the problem could be that users have installed node-sass globally and install sass locally just for that one project. This means, because svelte-preprocess tries to find node-sass first, the node module resolution algorithm would still find the global node-sass first and still throw the error about incompatible node versions.

Describe the solution you'd like
If acceptably for you, I would like to change the order of lookups from ['node-sass', 'sass'] to ['sass', 'node-sass'].

Describe alternatives you've considered
#149 seems to add an option to use a specific compiler, that could help, too.

How important is this feature to you?
Not super important because there are workarounds, but it would still help us.

@kaisermann kaisermann self-assigned this Jun 5, 2020
@kaisermann kaisermann added the enhancement New feature or request label Jun 5, 2020
@kaisermann
Copy link
Member

Hey @dummdidumm 👋 Yeah, I'm interested in changing it. In fact, I tried in #151 but started to get some errors regarding @imports not being able to resolve their paths. I'll merge #149 soon (with the option implementation instead of compiler), but I'd also like to require sass first, I just didn't manage to get what was happening with the tests.

@kaisermann
Copy link
Member

@dummdidumm Just released v3.8.0 with the implementation prop! I'll try reordering the sass lookup again anyway 😁

@kaisermann
Copy link
Member

kaisermann commented Jun 5, 2020

When I switch from node-sass to sass I start getting this error:

image

Already tried to debug it, but I've hit a wall.

Maybe related to: sass/dart-sass#703.

Edit:

Yeah, I'm unable to reproduce the error outside of the testing environment 🤔 🤔 🤔

@dummdidumm
Copy link
Member Author

Now that the implementation prop is implemented, could you use that in the tests to hardcode it to use node-sass so it works like before?

@kaisermann kaisermann added the next-major will be fixed in the next major label Jul 6, 2020
kaisermann added a commit that referenced this issue Jul 6, 2020
@kaisermann
Copy link
Member

Will be changed in v4 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request next-major will be fixed in the next major
Projects
None yet
Development

No branches or pull requests

2 participants