Sass includePaths and Babel-resolved aliases

orlin commented 7 years ago

I can’t get eyeglass to work and that seems like the best way to load sass from node_modules. Perhaps the only way in some cases it seems (when reaching for a path inside a module is necessary). For example trying to load scss from material-components-web fails, very sad. I was hoping eyeglass could help with that and other things. There is an open issue that I just commented on. Also tried the webpack-combine-loaders from the example @wesm87 has there which didn’t help. Any ideas about how to get this working? Look like a very worthwhile addition / a best practice?

davibe commented 7 years ago

I don't completely understand your issue.

It seems like you want to load a sass file from inside node_modules directory. If this was possible even without eyeglass would this fix your issue ?

If so please have a look at https://github.com/davibe/next.js-css-global-style-test/issues/2

orlin commented 7 years ago

Yes, I know about normalize. There is also normalize-scss that I do @import '~normalize-scss’; from node_modules without eyeglass. Eyeglass lets you reach it without a ~ (which may not be supported in webpack 3 thus also would be breaking sass-loader I presume), and eyeglass it seems can import without a relative path, and also with any arbitrary path following the module name.

I just tried a relative path to one of the material-components-web modules: @import '../node_modules/@material/button/mdc-button.scss’; and it doesn’t work - from the pages directory, and neither does @import '~@material/button/mdc-button.scss’; with a ~ rather than relative path. The problem is that further imports from that scss to another of its related modules (notice also without .scss extension) don't work. I guess that material-components-web are also expecting eyeglass-resolved paths or other build tools knowing how to work within node_modules. Here is the error - all the same no matter how I import it:

 error  in ./pages/index.js.scss

Module build failed:
@import "@material/animation/variables";
^
      File to import not found or unreadable: @material/animation/variables.
Parent style sheet: /Users/om/Dev/mel/drmelgill.com/node_modules/@material/button/mdc-button.scss
      in /Users/om/Dev/mel/drmelgill.com/node_modules/@material/button/mdc-button.scss (line 17, column 1)

 @ ./pages?entry 43:15-41
 @ multi ./~/next/dist/client/webpack-hot-middleware-client ./pages?entry

There is also the babel-plugin-module-resolver that could help, I had it planned to try that today / tomorrow. Will let you know how it goes. So I could just import with ‘node_modules/…’ or ‘modules/…’ without knowing the relative distance. That’s pretty cool for other things too like ‘components’, etc. I doubt it will help in the case of sass modules though.

And in any case there is a lot more to eyeglass than imports, and many sass modules on npm supporting it. From memory: there's also support for images, sprites, etc. I think it would be a very worthwhile addition, if someone could figure out how to plug it in...

orlin commented 7 years ago

Something else: the failing @material/animation/variables that node_modules/@material/button/mdc-button.scss tries to import is actually a node_modules/@material/animation/_variables.scss as I just remembered to check. This is something the babel resolver wouldn't know how to find (the prepended file _) whereas I think that is considered normal in sass context.

orlin commented 7 years ago

A nice guy from material-components-web helped me solve this issue. So we can let eyeglass worry about its webpack compatibility. If you look at the solution though, I think your next.js example could be improved by setting the sass-loader's includePaths to all node modules. Because we don't depend on any specific modules in the example, all possible paths would be the only thing that makes sense. I think though that what I currently have working presupposes use of yarn, so maybe at least add a comment in the #readme? Here is what changed about my next.config.js so far:

const path = require('path')
const glob = require('glob')

//...

      { test: /\.s(a|c)ss$/,
        use: [
          'babel-loader', 'raw-loader', 'postcss-loader',
          { loader: 'sass-loader',
            options: {
              includePaths: glob.sync('node_modules').map((d) => path.join(__dirname, d))
            }
          }
        ]
      },

orlin commented 7 years ago

For the record, here is my improved config:

          { loader: 'sass-loader',
            options: {
              includePaths: ['node_modules', 'node_modules/@material/*']
                .map((d) => path.join(__dirname, d))
                .map((g) => glob.sync(g))
                .reduce((a, c) => a.concat(c), [])
            }
          }

The example could come without the 'node_modules/@material/*'- then people would add whatever paths or globs they want to the array.

davibe commented 7 years ago

Interesting, could you work on a PR ?

orlin commented 7 years ago

On this repo or next.js/examples/with-global-stylesheet?

davibe commented 7 years ago

i think so, yes

orlin commented 7 years ago

Sure, I’d love to do my first PR on Next.js, thank you for leading the way! Here is what I intend to include in this one:

refactor of next.config.js to use the newer Webpack 2 rules style
load either .scss or .sass
includePaths for sass-loader - global styles and node_modules
move the stylesheets to a /styles dir (so that relative path imports won’t be necessary neither from scss nor from js) and rename them to styles/index.scss and styles/index.css - I’ve been happily doing this at the level of components and pages (what we’re doing here), and I guess at the layout (or even custom _document) level is just as possible for even more global stylesheets (though @include works fine, making further globalization optional) - performance optimization is a separate issue I haven’t thought about yet
setup an alias to styles using babel-plugin-module-resolver
material-components-web is out of scope as far as this example, but I’ll leave the glob feature with a note in the #readme

I have at least one further improvement in mind regarding PostCSS. Let’s discuss / do that with a separate issue…

orlin commented 7 years ago

The pull request I just made has one flaw that I can see - example.gif doesn't reflect that the css file has been moved and renamed. Technically it could have stayed in pages the way it was. I just thought it would be better for consistency...

davibe commented 7 years ago

I will update things accordingly.

davibe commented 7 years ago

@orlin i have brought in the changes from next.js but in order to make it work i had to do this https://github.com/davibe/next.js-example-with-global-stylesheet/commit/f1ec0753b0703f93f591b4f4ce6e45f38dd78e4e

Does this mean the example in the next.js sources is broken ? or what am i missing ?

orlin commented 7 years ago

@davibe not all the changes. About styles without relative path you'd have to add the following to .babelrc's plugins:

    [
      "module-resolver", {
        "root": ["."],
        "alias": {
          "styles": "./styles"
        },
        "cwd": "babelrc"
    }]

I see you've got babel-plugin-module-resolver in the dependencies, so this should do it...

orlin commented 7 years ago

Here are the 2 pull requests, if you want to double-check that you copied everything over:

orlin commented 7 years ago

@davibe I also got a link on the example's readme fixed with this PR to what I originally intended -- Deconfusing Pre- and Post-processing

coderavels commented 4 years ago

How can this be achieved in Next@9.5 without creating custom next.config.js?

davibe / next.js-example-with-global-stylesheet

Sass includePaths and Babel-resolved aliases #3