Skip to content
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

Closed

Conversation

renestalder
Copy link

@renestalder renestalder commented Dec 7, 2018

  • CSS
    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.
  • Resolve image references
    Automatically resolve image references in JavaScript and CSS and move it either to one folder or keep file structure.
  • Resolve font files
    Automatically resolve font file references and move them to the fonts folder.

closes #34

Replaces the manual copy task with Webpack loaders
to resolve the CSS dependencies.
@CLAassistant
Copy link

CLAassistant commented Dec 7, 2018

CLA assistant check
All committers have signed the CLA.

@jpkempf
Copy link

jpkempf commented Dec 20, 2018

Just noticed this, nice work so far! I'm happy to do some testing once you're done.

@mbulfair
Copy link
Member

@renestalder this looks good, apologies for the delay. Should wait to merge until all 3 items are complete, or split this up?

@renestalder
Copy link
Author

@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.

@mbulfair
Copy link
Member

mbulfair commented Jan 9, 2019

@renestalder be aware I've published some updates, they probably won't break anything. 🤞

@mbulfair mbulfair added the 🏗️ WIP Not ready to be merged, still work in progress. label Jan 9, 2019
@renestalder
Copy link
Author

@mbulfair Thanks for the note :)

@ItsJepser
Copy link

Do you know when this will be finished? This will be very helpful for me.

@renestalder
Copy link
Author

@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.

@renestalder
Copy link
Author

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 @import to define your bundles, similar to how imports work in Less and Sass. Every file inside the define JavaScript and CSS folder is an entry file, allowing you to just create new files if you need different bundles (e.g. polyfill bundles, print css and so on).

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.

@renestalder
Copy link
Author

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.

@mbulfair
Copy link
Member

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",
Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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

Copy link
Author

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.

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

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.

Copy link
Member

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.

@mbulfair mbulfair changed the base branch from master to latest January 31, 2019 18:42
@mbulfair mbulfair closed this Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗️ WIP Not ready to be merged, still work in progress.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace copy configuration with automatic copying via Webpack
5 participants