Skip to content
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

feat: ๐ŸŽธ add scss support for tilde (~) imports #278

Merged
merged 9 commits into from
Nov 17, 2020

Conversation

kaisermann
Copy link
Member

@kaisermann kaisermann commented Nov 7, 2020

โœ… Closes: #277

edit 1:
Just gotta do a test for it ๐Ÿ˜

edit 2:
Tests are failing now, not sure why. See sass/node-sass-middleware#104 Can't use the done for renderSync runs.

Current challenge:
it seems the sass compiler doesn't pass an importer's returned value through the includePaths paths, so it needs to return an absolute path. We use the url and prev (source) arguments to resolve it , but, for some reason, the prev value is coming as stdin instead of the importer path.. Needed to pass file alongside the rest of the options.

edit 3:
Need to rewrite that importer, the logic is wrong. It will stop on the first node_modules found, even if there's no package with the specified path.

References:
https://github.com/matthewdavidson/node-sass-tilde-importer/blob/master/index.js
https://sass-lang.com/documentation/js-api#importer

@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from 61e15a2 to 74d3d8b Compare November 7, 2020 15:23
@kaisermann kaisermann added the help wanted Extra attention is needed label Nov 8, 2020
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from 241d1ec to cc65464 Compare November 8, 2020 13:29
@kaisermann kaisermann removed the help wanted Extra attention is needed label Nov 8, 2020
@kaisermann kaisermann self-assigned this Nov 8, 2020
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch 15 times, most recently from eafe7c9 to d2dc63b Compare November 8, 2020 14:36
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from d2dc63b to 4a270c9 Compare November 8, 2020 14:38
@kaisermann
Copy link
Member Author

Not sure why the windows test is failing ๐Ÿ˜ข

@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch 2 times, most recently from 3e69e25 to 4a270c9 Compare November 9, 2020 14:11
@kaisermann kaisermann added the help wanted Extra attention is needed label Nov 9, 2020
@kaisermann kaisermann marked this pull request as draft November 9, 2020 17:20
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch 4 times, most recently from 5cbcf7c to 665f1fa Compare November 10, 2020 13:02
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from 665f1fa to 360790a Compare November 10, 2020 13:05
@ghost
Copy link

ghost commented Nov 16, 2020

@kaisermann Hi, do you know why the windows test isn't working? I didn't even know certain NodeJS modules/features are platform-dependent ๐Ÿ˜ฎ

@kaisermann
Copy link
Member Author

@GitGangGuy Nope, no idea. The file is there ๐Ÿคท However, it may work if I read it manually and use the { content } return API instead of { file }.

@kaisermann kaisermann marked this pull request as ready for review November 17, 2020 12:20
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from 7d92077 to 2026a3f Compare November 17, 2020 13:19
@kaisermann kaisermann force-pushed the feat/support-scss-tilde-import branch from 2026a3f to 215bcbf Compare November 17, 2020 13:19
@kaisermann kaisermann merged commit 39c3cb8 into main Nov 17, 2020
@kaisermann kaisermann deleted the feat/support-scss-tilde-import branch November 17, 2020 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add an import resolver for tilde (~) node_modules imports
1 participant