gonzofish / angular-librarian

An Angular 2+ scaffolding setup for creating libraries
https://www.npmjs.com/package/angular-librarian
MIT License
91 stars 9 forks source link

Importing 3rd Party SCSS #37

Closed gonzofish closed 7 years ago

gonzofish commented 7 years ago

Follow-on from this comment on #27

tommueller commented 7 years ago

@gonzofish just played around with this and I cannot use any @import within the scss files. I get the error also when I add the file that should be imported into the components folder directly.

Would really like to help you out with a PR, but I do not really understand this project in the first place, sorry :man_facepalming:

gonzofish commented 7 years ago

After some digging around and testing, I might have a fix (based on this comment), although I can't really tell.

In my sample project I @imported two local files (1 css, 1 scss) and one of the Angular Material prebuilt themes CSS. I also created a node module and imported that into my component and it worked. So like I said, I think it will work.

I'll close this issue when the change has been published.

gonzofish commented 7 years ago

The original issue was finally found...we're using node-sass to convert SASS files to CSS before inlining them in components. Part of the problem is that node-sass doesn't really do @import well. You have to specify an importer attribute when rendering the SASS to CSS, like:

            importer: (url, prev, done) => {
                if (/^~/.test(url)) {
                    url = path.resolve('node_modules', url.slice(1));
                } else {
                    url = path.resolve(srcDir, url);
                }

                return { contents: fs.readFileSync(url, 'utf8') };
            }

Basically we're going to look for a leading ~ and, if so, we'll replace that leading ~ with node_modules and read the file from there. Otherwise, we make the URL relative to the component's directory (this should let ../ work too) and read the file from there.

gonzofish commented 7 years ago

This is all going to be happening in the tasks/inline-resources.js file at line 76.

gonzofish commented 7 years ago

Tested locally and seems to work.

Here's the release

tommueller commented 7 years ago

Thank you so much for your effort! It kind of works for me, but not really ...

I can successfully import .css files now, but cannot import .scss-files. I still get the same error with .scss-files. I forked the repo, because I thought it's probably an easy fix, but I saw that you did include .scss everywhere. Did you test it with .scss-files?

Using ngl serve everything works fine!

Cheers, Tom

gonzofish commented 7 years ago

Hi Tom -

I did test it with both CSS & SCSS files. I'm obviously missing something here...your import looks like:

@import '~package/dist/scss/imports';

Correct? Imports, I take it, is actually imports.scss?

tommueller commented 7 years ago

Here is a list with options I tested and with the result (with an css and scss in the parent folder, and in the node_modules/package folder:

so basically everything with scss is not working for me and with css it does ...

gonzofish commented 7 years ago

Thanks for doing this work. I haven't had trouble with files with an .scss extension, but did have issues with files with no extension (which I'm working on fixing).

Can you look in tasks/inline-resources.js line 76 and see if it looks like this one in the repo?

gonzofish commented 7 years ago

Also, I'd never used partials in Sass before, so that'll take some work as well

tommueller commented 7 years ago

Can you look in tasks/inline-resources.js line 76 and see if it looks like this one in the repo?

Yes, looks exactly like it!

gonzofish commented 7 years ago

Ok...thanks. Looking into it. Appreciate the patience.

gonzofish commented 7 years ago

Do the partials work when you use ngl serve?

tommueller commented 7 years ago

Do the partials work when you use ngl serve?

yes

gonzofish commented 7 years ago

@tommueller just a heads up, on a local project, it seems to be working for both serve & build. Here's the sample Sass file I have:

@import "~fake-node-lib/fake.css";
@import "~fake-node-lib/fake-scss";
@import "local.scss";
@import "../partial";

.test {
    color: $text-color;
    height: 200px;
    width: 200px;
}

The variable $text-color gets imported from _partial.scss a level above and other stylings get pulled in from the other stylesheets.

I've also tried all the the Sass files with a .scss extension on them as well.

gonzofish commented 7 years ago

I've published 1.0.0-alpha.16 which tries to solve this problem. I'll leave it open until we get a confirmation from you @tommueller

tommueller commented 7 years ago

Thanks a lot @gonzofish

I will only be able to check on this on Monday. Will get back to you as soon as I tested it!

gonzofish commented 7 years ago

Not a problem. Thank you for being patient and working with me on this.

tommueller commented 7 years ago

@gonzofish great this works for me now! Thank you very much!

I did have to initialize a whole new project though, could not get the original one to work after upgrading to alpha.16 ...

gonzofish commented 7 years ago

Great to hear it worked. Do you remember what exactly wasn't working?

gonzofish commented 7 years ago

Did you remove the stack trace? I see it in the email but not the issue here...

tommueller commented 7 years ago

In my old project I still got the same error as posted here: https://github.com/gonzofish/angular-library-set/issues/27#issuecomment-304660371

I removed the stack trace, that error was unrelated (caused by a VSCode-plugin).

gonzofish commented 7 years ago

I know this is a silly question, but you did upgrade the older project? The reason for that error is that it is saying it cannot find the file specified. On a re-init'd project (sorry, that's the "update" method for now), this should only happen with the @import-ing of a .css file that is missing.

tommueller commented 7 years ago

Yes, I did upgrade ;) or at least I did this:

I think that should've done it, right?

gonzofish commented 7 years ago

That would install alpha.16, but you'd still need to run angular-librarian's init to re-scaffold the project using the latest. I guess since I know at least 1 person is actively using this I should investigate a better migration/update strategy from version-to-version.

tommueller commented 7 years ago

Okay, I did not know and do that!

Wasn't a problem, because it was a one component library so far, but very good to know for the future!

gonzofish commented 7 years ago

Ok so this is officially closed, but has spawned a new issue for better migration: #38