Skip to content

WIP: Font awesome fix for npm > 3#84

Open
jure wants to merge 2 commits into
substance:masterfrom
jure:font_awesome_fix
Open

WIP: Font awesome fix for npm > 3#84
jure wants to merge 2 commits into
substance:masterfrom
jure:font_awesome_fix

Conversation

@jure

@jure jure commented Jan 9, 2016

Copy link
Copy Markdown
Contributor

This: https://github.com/substance/lens/blob/master/styles/_lens.scss#L2 doesn't work anymore in Npm > 3. Npm > 3 flattens the dependency tree, and that means that if the lens module is installed as a dependency, then it will no longer have a nested node_modules folder with its dependencies, but those dependencies will instead be installed at the project's root node_modules.

I'm not exactly sure what the absolute best fix is, so this is a start. There is currently one Webpack specific fix at: https://github.com/jure/lens/blob/16928130bc84902482ca16afb1e558b5bd76ec39/styles/_lens.scss#L1
so this one will have to be changed to a generic solution for sure.

I imagine you do want to fix this, but if you don't want to support npm > 3, I suggest you make that clear in your package.json, by specifying a version in the engines.

@michael

michael commented Jan 13, 2016

Copy link
Copy Markdown
Member

Is this actually possible?

@import 'substance/packages/_all';

Like does this resolve in an npm-ish manner?

@jure

jure commented Jan 18, 2016

Copy link
Copy Markdown
Contributor Author

No, sorry. This SCSS stuff is quite confusing. So SCSS does not have a way to distinguish relative paths from other imports: https://www.npmjs.com/package/sass-loader#imports

It's important to only prepend it with ~, because ~/ resolves to the home-directory. webpack needs to distinguish between bootstrap and ~bootstrap because CSS- and Sass-files have no special syntax for importing relative files. Writing @import "file" is the same as @import "./file";

It does if you're using webpack's sass-loader though, so that's why I assumed.

The bigger issue is that there is currently no way that I know of, that would enable me to use your current font-awesome integration without forking the project and using webpack specific code, such as the ~ symbol for the $fa-font-path. I'm digging into this issue now, as it will surely pop up with multiple external modules.

@jure

jure commented Jan 18, 2016

Copy link
Copy Markdown
Contributor Author

This is how I configure font-awesome for Lens now:
https://gitlab.coko.foundation/pubsweet/substance-components/blob/master/styles/reader.scss#L1

This works for NPM < 3, but will fail for NPM > 3. I can't see how we can make this be compatible with both versions, as the line for NPM > 3 will have to be:

@import "~font-awesome/fonts";

Which fails in NPM < 3. Ideas?

@michael

michael commented Jan 18, 2016

Copy link
Copy Markdown
Member

I think we can configure the SASS include path. I think that will be the way to go. Use whatever works for you atm. and we will fix that in a cleanup iteration together so it works for all the cases.

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.

2 participants