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

Overwrite sass imports with parcel's resolver #1256

Merged
merged 11 commits into from Apr 27, 2018

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Apr 26, 2018

Uses an experimental node-sass feature for overwriting the imports, it seems to be stable and very usefull after some testing

Closes #1182 #39

@codecov-io
Copy link

Codecov Report

Merging #1256 into master will increase coverage by 2.52%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1256      +/-   ##
==========================================
+ Coverage   85.86%   88.38%   +2.52%     
==========================================
  Files          77       77              
  Lines        4307     4192     -115     
==========================================
+ Hits         3698     3705       +7     
+ Misses        609      487     -122
Impacted Files Coverage Δ
src/assets/SASSAsset.js 96.77% <87.5%> (+3.15%) ⬆️
src/transforms/babel.js 88.55% <0%> (-6.97%) ⬇️
src/visitors/dependencies.js 89.13% <0%> (-5.44%) ⬇️
src/visitors/globals.js 92.3% <0%> (-3.85%) ⬇️
src/assets/GLSLAsset.js 93.1% <0%> (-3.2%) ⬇️
src/transforms/uglify.js 71.73% <0%> (-2.18%) ⬇️
src/utils/config.js 86.95% <0%> (-0.35%) ⬇️
src/assets/GraphqlAsset.js 100% <0%> (ø) ⬆️
src/assets/VueAsset.js 87.5% <0%> (+0.83%) ⬆️
src/assets/LESSAsset.js 100% <0%> (+2.12%) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d984a5...03967cd. Read the comment docs.

resolver.resolve(url, prev === 'stdin' ? this.name : prev)
).path;
} catch (e) {
resolved = url;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this fall back to the default SASS resolver when Parcel doesn't find anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this falls back to the default sass resolver. Mainly for built-in sass modules.
Tested this with the example repo of #1182 . In this example bootstrap uses mixins which is catched by the sass resolver

@devongovett
Copy link
Member

Awesome, this must be a new feature! I think it will solve a lot of issues, e.g. #39. Let's go through and find other issues related to sass resolution and see if this fixes them too.

@devongovett devongovett merged commit 31190cf into master Apr 27, 2018
@devongovett devongovett deleted the feature/custom-sass-importer branch April 27, 2018 14:20
devongovett pushed a commit that referenced this pull request Oct 15, 2018
devongovett pushed a commit that referenced this pull request Oct 15, 2018
@BennyAlex
Copy link

BennyAlex commented Jan 22, 2020

@devongovett @DeMoorJasper whats about #1280 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants