patternfly / patternfly-3

This repo contains the HTML, CSS, and JQuery for the PatternFly 3 project.
https://www.patternfly.org/v3
MIT License
1.15k stars 239 forks source link

npm module for PF not transpiling LESS to CSS properly #458

Closed kahboom closed 8 years ago

kahboom commented 8 years ago

Hello! We're using PatternFly for the front end of the iPaaS project (here, my dev branch here). Our stack is Angular 2.0 and Webpack 2. I'm aware that Angular 2 is not yet supported, as I opened an issue in the angular-patternfly repo here, but would still like to use the regular PatternFly CSS. I’m getting an error when I try to load the PF npm module, see the Gist with the error here.

It appears to be an issue with the CSS being imported, even if I try to load just the CSS directly with require() when using node.js. Upon further inspection, the normal CSS file looks like a LESS file with @ symbols, etc.

It also happens with the patternfly-additional.css file, both the full and minified versions. I can probably just require() the LESS files directly, but I would have to do that for all of them one by one and figure out which I need, since I don’t see a file that imports them all.

We are trying to avoid using Bower completely. Worst case scenario I can either require the LESS files manually, use a CDN hosted CSS, download a copy of PF and host it locally (though versioning would drive me nuts), or test angular-patternfly's CSS -- other than that, any suggestions? Thought it might be good to report it in case it's an unknown issue. Thanks in advance!

priley86 commented 8 years ago

As you're probably noticing, pathing with Webpack can be challenging. As it's a new technology, there are still a lot of open issues.

One thing I noticed while attempting to compile Patternfly LESS directly from the NPM node_modules folder was the relative paths in LESS import statements were not always respected. This appears to be an open issue with Webpack less-loading. https://github.com/webpack/less-loader/issues/76

For the time being, I would suggest copying our LESS files into your project source (using an additional build step) so that paths are respected, or simply copying our distributed CSS directly to your build output (using the CopyWebpackPlugin) and extending our CSS where necessary. These are just suggestions and notes I've made while testing Patternfly with Webpack, but please do let us know if you find any other solutions.

There are some examples of how to do this and a sample Webpack setup in our Patternfly Demo App. https://github.com/patternfly/patternfly-demo-app

Of course, another alternative may be to look at Gulp as a build tool. Webpack is on the bleeding edge right now, so of course there will be challenges.

Hope this helps.

kahboom commented 8 years ago

Thanks @priley86 - Your response helped a lot, as did the patternfly-demo-app. It's a shame that 1) LESS files can't be imported into SCSS files and 2) Webpack's LESS loader has relative path issues.

I'm following the examples provided in the patternfly-demo-app to load PF JS files in a similar way. I provided some feedback for the patternfly-demo-app and opened up an issue here, so will hopefully be creating a PR sometime this week.

I decided to go with your suggested "copy distributed CSS" route, as I'd really rather not copy the LESS files manually and forget they're there later on. But I didn't realize that there seems to be an issue with the extract-text-webpack-plugin npm package referenced in the webpack.common.js file here. I'm getting an error from the Webpack library about a deprecated method being used by that library (you can see the gist here) -- which is likely due to the fact that I'm using Webpack 2 (living life on the bleeding edge here).

Anyway, totally not your problem and I see that there is already an issue open for that here, so for now I'll use CDN-hosted CSS for PF (which seems to be working just fine for dev purposes) and see if there's anything I can help with on their end as well.

Theoretically, another option would be to use a less transpiler package in a prestart npm script and require that transpiled CSS instead, but I might just be too lazy to try that. Okay, I'm done rambling now. Thanks again!

jeff-phillips-18 commented 8 years ago

@kahboom Please take a look at https://github.com/patternfly/patternfly-sass . It contains SCSS versions of the patternfly less files. Might be helpful to you.

kahboom commented 8 years ago

@jeff-phillips-18 - Thanks! It looks like it was made for use with Ruby on Rails, but I'll just install it with npm by referencing the GH repo and use the tags for version control. There used to be an issue with installing GH repos and using npm's shrinkwrap, but I think that was fixed already if I recall correctly.

priley86 commented 8 years ago

@kahboom great to see you are experimenting with Webpack2. I'm pretty excited about tree-shaking features coming in, but haven't yet tried them. Would be happy to accept any contributions you might have on that front, and really appreciate this!

Are we safe to close this issue? Will try to comment now on #10...

kahboom commented 8 years ago

@priley86 - Yep, feel free to close it. I'll report back with any miseries or successes with using Webpack 2 and PF together (in conjunction with Angular 2). I might write a blog post on it this/next week if I get a chance as well, in case anyone else finds it useful. Thanks so much for all the help!

ncoghlan commented 7 years ago

@kahboom Were you able to get this working without modifying the upstream PatternFly CSS files? I'm hitting similar problems now along the lines of:

    ERROR in ./node_modules/patternfly/dist/css/patternfly.css
    Module parse failed: /home/ncoghlan/devel/python-release-latency/node_modules/patternfly/dist/css/patternfly.css Unexpected character '@' (2:0)
    You may need an appropriate loader to handle this file type.
    | /* PatternFly */
    | @font-face {
    |   font-family: "Open Sans";
    |   font-style: normal;
     @ ./node_modules/style-loader!./node_modules/patternfly/dist/css/patternfly.css 4:14-43

As far as I can tell, I've set https://github.com/ncoghlan/python-release-latency/blob/master/webpack.config.js up correctly based on the demo, but running ./node_modules/.bin/webpack --config webpack.config.js fails when the CSS files are included from https://github.com/ncoghlan/python-release-latency/blob/master/assets/js/index.js

ncoghlan commented 7 years ago

Success! My initial problems turned out to be a combination of two things:

  1. The syntax for configuring ExtractTextFiles has changed from what it was when this demo was first written
  2. When an appropriate loader is configured based on the file extension, specifying it again in the requires call will cause problems, so you shouldn't do that

Those fixes were applied here: https://github.com/ncoghlan/python-release-latency/commit/66461da8964e0d548128fd61c4ef2349d23d42c9

So these three files are giving me what appears to be a fully functional webpack 2 based set of PatternFly assets: