-
Notifications
You must be signed in to change notification settings - Fork 94
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
WIP: Resolve asset dependencies via Webpack loaders and resolvers #35
WIP: Resolve asset dependencies via Webpack loaders and resolvers #35
Conversation
Replaces the manual copy task with Webpack loaders to resolve the CSS dependencies.
Just noticed this, nice work so far! I'm happy to do some testing once you're done. |
@renestalder this looks good, apologies for the delay. Should wait to merge until all 3 items are complete, or split this up? |
@mbulfair I feel this is only useful with all 3 resolved. So wait with the merge. I'll clearly try to finish this in the next couple of weeks. |
@renestalder be aware I've published some updates, they probably won't break anything. 🤞 |
@mbulfair Thanks for the note :) |
Do you know when this will be finished? This will be very helpful for me. |
@ItsJepser I most likely work on Fridays on this and I estimate this doesn't take much time as It's simply a copy/paste job from my running projects. I plan to finish this until the end of the month at latest. |
Give it a spin and test it. I actually wanted to go a bit further, removing the need to define css/js folders all together. This also works at least for JavaScript, but not for CSS yet. Anyway, at least you can use CSS It's a setup I use quite often, but I'm not sure that is the way other people work in patternlab. Also I have additional tasks in my forks to create package bundles, e.g. for delivery/release to be used by other developers. I didn't at that, because it's too much on top of Patternlab. |
Ah, and a note: I added tests. I added a folder where one can create test files and in travis ci you can define all folders that should be tested. During travis build, the files from those folders get merged into the source folder before a build is made. This is giving us a basic test if the build run through and in addition a foundation for e2e tests if required. |
Thanks @renestalder i'll take a look as soon as I get some free time. Hopefully soon, this is incredible work. |
"event-hooks-webpack-plugin": "^2.1.1", | ||
"extract-text-webpack-plugin": "^4.0.0-beta.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is on webpack 4, I would probably have use https://github.com/webpack-contrib/mini-css-extract-plugin instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbulfair The Mini CSS Extract Plugin doesn't fulfill the needs here. I actually tried for quite a while to get it working with Mini CSS Extract Plugin, but it's never as powerful as the Extract Text Webpack Plugin (as the name implies).
extract-text-webpack-plugin on version 4 is compatible with Webpack 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What needs does mini fall short? Granted I've stuck with mini in all my configurations since webpack 4 came out. Just curious what it doesn't do in this case @renestalder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSS Mini Extract is only made for CSS extraction to files, where Extract Text Plugin is made for general text extraction to files.
But I just saw that maybe for the CSS part alone I could go with the Mini CSS Extract Plugin, just to fulfill what they recommend in the readme of the Extract Text Plugin.
Since webpack v4 the extract-text-webpack-plugin should not be used for css. Use mini-css-extract-plugin instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbulfair I'll check again if I get it working with CSS Mini Extract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renestalder did you get a chance to update this? Also I pushed some updates to latest and will need the conflicts resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbulfair Not yet as this version is currently running without issues in my projects so I actually didn't have a chance/reason to try it with Mini CSS extract version.
But it's on my radar. Just couldn't justify the time to put into it yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@renestalder reach out on gitter when you can.
All files in the defined CSS folder are entry points by default. In addition, it's now possible to store CSS outside of the predefined CSS folder and use CSS'
@import
feature to bundle those files in one of the entry point files in the CSS folders. This for example allows to store CSS along the patterns and components.Automatically resolve image references in JavaScript and CSS and move it either to one folder or keep file structure.
Automatically resolve font file references and move them to the fonts folder.
closes #34