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

Idea for preprocessors #2061

Open
ahukkanen opened this Issue Mar 12, 2015 · 29 comments

Comments

Projects
None yet
@ahukkanen
Copy link
Contributor

ahukkanen commented Mar 12, 2015

Just throwing an idea around here, as I've seen some people in the forums wanting to use Sass instead of Less as the CSS preprocessing language. What about integrating something like Sprockets in rails which happens to have a port for PHP:
https://github.com/vendethiel/Sprockets-PHP

That would take care of that and allow people to use either one of them + generalize those under an abstraction layer.

In rails I find it quite useful as if I want to use a Less-based UI library, only thing I need to do is swap out the preprocessing engine.

And this would somewhat also make c5 more future proof if Sass happens to win the fight between the two.

EDIT:
Adding another option also for handling the assets:
https://github.com/kriswallsmith/assetic/

UPDATE:
There is a working demo implementation (using Assetic) for this available at:
https://github.com/mainio/c5_asset_pipeline

@MrKarlDilkington

This comment has been minimized.

Copy link
Contributor

MrKarlDilkington commented Mar 13, 2015

+1

There are a lot of Sass users who would appreciate something like this.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Mar 13, 2015

One more benefit about this is that I think it would be easier to combine JS files also if we had something like this. I think it's currently a bit awkward to combine the site-specific JS files through those includes in page_theme.php.

@Remo

This comment has been minimized.

Copy link
Contributor

Remo commented Mar 16, 2015

@ahukkanen keep in mind that we're currently using oyejorge/less.php and not leafo/lessphp. The latter had some issues with bootstrap, not sure if they were ever fixed.. leafo was quite a bit faster though..

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Mar 16, 2015

@Remo Couldn't the underlying library be changed to better suit c5's needs?

I don't think this looks too complicated to change:
https://github.com/vendethiel/Sprockets-PHP/blob/master/lib/Sprockets/Filter/Less.php

@Remo

This comment has been minimized.

Copy link
Contributor

Remo commented Mar 16, 2015

@ahukkanen sure thing, just pointing out that we might have some more work to do before this library could be used!

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Aug 24, 2015

Also pointing here what @hissy pointed out at the forums:
http://www.concrete5.org/community/forums/5-7-discussion/less-is-a-lot-more-work/

Bootstrap 4 development moved from less to sass.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Aug 28, 2015

Here's another interesting looking library for the same purpose:
https://github.com/kriswallsmith/assetic/

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Aug 28, 2015

We might be needing to work on a project with Sass files in the near future, so if we could push this issue into some conclusion, we might be able to put in some hours or even days into this.

Not promising anything here, just saying that it might be in our interest (and probably many others' as well) to get something like this into c5.

Ping @aembler @KorvinSzanto

@aembler

This comment has been minimized.

Copy link
Member

aembler commented Sep 3, 2015

We wouldn't be averse to including a choice in the core – but keep in mind, the ONLY reason to include such a thing in the core would be to allow it to work with the style customizer. That's the only reason that we have LESS support in the core. Otherwise, just use whatever preprocessor you want and compile down to CSS. The reason for the LESS compiler in the core is so that we can use LESS files as the basis for the style customizer. If someone wants to use SASS files as the basis for the customizer that's fine, but every single current style customizer variable type needs to compile from SASS transparently otherwise it's a non-starter. So there has to be a PHP LESS library that can compile SASS files on the fly and give us access to the values, which we can parse into our own objects. It's not enough to just be a black box SASS compiler that compiles SASS into CSS. We need to work with it programmatically.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Sep 3, 2015

Makes sense didn't think of that aspect.

But I think it might still be possible keeping that in mind. I can investigate this a bit further.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Dec 28, 2015

Closed for...?

@KorvinSzanto

This comment has been minimized.

Copy link
Member

KorvinSzanto commented Dec 28, 2015

Leafo makes an scss -> css compiler that may work, I remember that we specifically chose NOT to use leafo's less compiler due to perfomance issues though: https://packagist.org/packages/leafo/scssphp

@ahukkanen I assume this was closed because it appeared stale, aembler just updated the tagging system and did some year end cleaning.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Dec 29, 2015

@KorvinSzanto Yes, I think the Leafo's compiler works well but it's not the point regarding this issue. This issue is about abstracting the whole thing, so that if the developers over the world decide next year that scss and less both are not good anymore and e.g. Stylus is the "next cool thing", we wouldn't be stuck with any particular language(s).

This may appear stale at the moment as we have many other things to focus on but I can assure you that we have done some work on this and we will be putting a working proposal of this to GitHub hopefully within the next couple of months. It will be a package but it shows how this could be done in the core as well.

@mnakalay

This comment has been minimized.

Copy link
Contributor

mnakalay commented Dec 29, 2015

Hey all,
Just my 2 cents. I have an add-on i the marketplace (Buttons Pro) that uses the core less compiler but was originally writtent to use Leafo scss compiler. Here's what I can say about the experience:
1- The core less processor and leafo scss processor compiled my stylesheets (big one) in about the same time (11 seconds) I didn't see any real difference in time
2- Leafo worked perfectly for almost everybody while testing the package in the PRB but then it failed for 2 users including JohnTheFish. It would just fail silently without an error message. John and I spent 2 weeks trying everything and were unable to make it work on his system (he made it super conservative but still) so I switched to less
3- Leafo doesn't work for newer versions of scss so for some newer pieces of code there is a need to rewrite it.
4- Less is REALLY a lot more work. As long as it's just for the theme customizer than by all means, Less is fine but for other purposes, what a pain in the rompus. And that's from someone who has to support a LESS based package.

I don't think it makes sense to have a CMS based on Bootstrap and not offer scss processing in the core when Bootstrap is moving away from less. I remember how annoying it was that Concrete5 used to use a modified version of Jquery and we couldn't just use a newer versions. Now if C5 doesn't move towards integrating scss we're going to be in a similar situation where bootstrap will have to be rewritten in less for Concrete5 and flexibility will be lost.

having said so, I am sure Bootstrap is going to use latest scss implementation and I don't think any fo the PHP scss processor does that yet.

@JohntheFish

This comment has been minimized.

Copy link
Contributor

JohntheFish commented Dec 29, 2015

@ahukkanen's case for an abstraction layer for preprocessors is good future-proofing, even if the only current usage for it is to implement the existing LESS preprocessor.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Jan 11, 2016

Just an update, here's what we have done currently:
https://github.com/mainio/c5_asset_pipeline

It currently works as an external package (although it requires a couple core overrides) just as a proof of concept / point for discussion but we are also prepared to integrate it into the core in case this seems to make sense for other developers too.

It implements the Less and SCSS filters just to display its capabilities but in no ways is limited to these two. Under the hood it uses Assetic for most of the asset handling. There is no limitations e.g. on adding a CoffeeScript .coffee filter for JS in case someone needs that. The current implementation does not add that filter, though, because there is no pure PHP compiler for CoffeeScript currently available (because of which the installation would be more complicated with that enabled).

It also adds the possibility to have the theme's customizable style preset files in any preprocessing language, as pointed out by @aembler.

In the near future we will also be adding an example theme which shows exactly how to use it but the README file already provides the snippets on how to use it within the themes.

Currently most probably not 100% bug free and still work in progress but at least a good starting point.

@aembler @KorvinSzanto Can we reopen this issue? This is important for us at least and I believe many other developers would highly appreciate at least the SCSS filter, e.g. with any Bootstrap 4 based themes.

@MrKarlDilkington

This comment has been minimized.

Copy link
Contributor

MrKarlDilkington commented Jan 11, 2016

@ahukkanen

This looks really interesting. I know that many people would appreciate having the flexibility to use multiple preprocessors and compilers.

@aembler aembler reopened this Jan 11, 2016

@ArniPL

This comment has been minimized.

Copy link
Contributor

ArniPL commented Jan 15, 2016

+1 for using Assetic. Really adds flexibility.

@krebbi

This comment has been minimized.

Copy link

krebbi commented May 24, 2016

+1 for Assetic

@yoroshikun

This comment has been minimized.

Copy link

yoroshikun commented May 24, 2016

+1 This is by far a necessity for many developers out there and the upcoming release of bootstrap 4 (which concrete 5.7 - 5.8 uses bootstrap 3 normally) Is built on the SASS pre processing language rather than the LESS one which has become somewhat outdated (not to say its not good, just seems everything is going the SASS route) due to the rise of SASS. As a newer developer using the latest cutting edge technologies is a must, I chose concrete 5 as the CMS for that reason. Interesting enough though framework systems AKA. bootstrap 4 and foundation 6 now only support SASS as their pre processing language and depend on it, Sadly Concrete 5 still only supports less and bootstrap 3 naively for customization themes. Of course we can make custom themes that have the compiled minified css output of say foundation 6 and bootstrap 4 and import it like we do for the "custom" theme css & js but this cuts user dashboard customization and makes it hard for developers to make themes that have frameworks other than the supported bootstrap 3 one.

All in all adding support for Assetic (another great Asset system to replace the current one) and the asset pipeline that mainio has made really would allow for a more diverse range of developer to convert or start using concrete 5 due to its flexibility with 'native' SASS and other preprocessing languages support. Concrete will have a flood of developers that have previously been turned back by the limitation of less only work environment and this can only be a good thing for the community, (more diverse themes and plugins) And what is a better time to make a great change than the concrete 5.8 update, Its big and adding this would make the update even more appealing than the last.

Thanks For reading.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented May 24, 2016

Also, I think this should already be quite given but the asset pipeline system would not only allow different developers using different preprocessing languages for CSS (they can choose the one they prefer the most), but it would also allow people using preprocessing languages such as CoffeeScript for JavaScript as well.

The suggested pipeline approach implemented with Assetic would also not only allow choosing the preprocessing langauges but also allow different developers to use different preprocessing backends. Some people might e.g. occasionally need the cutting edge features from Sass that would not be implemented yet in the PHP processing library but would already be implemented e.g. in the node preprocessor. In these situations developers could simply swap the Assetic's filters to use another backend (different filters are already widely provided by Assetic).

@ob7

This comment has been minimized.

Copy link
Contributor

ob7 commented Sep 23, 2016

+1 for Assetic

@aembler aembler modified the milestones: Future, 8.0 Nov 7, 2016

@Biggisb

This comment has been minimized.

Copy link

Biggisb commented May 3, 2017

I realy would like to see SASS in action with the C5 Core Setup...

@Kaapiii

This comment has been minimized.

Copy link
Contributor

Kaapiii commented Aug 10, 2017

+1 for Assetic

@aembler aembler removed this from the Future milestone Mar 5, 2018

@JohntheFish

This comment has been minimized.

Copy link
Contributor

JohntheFish commented Feb 11, 2019

This has been added to the suggestions for Summer of code
https://www.concrete5.org/about/blog/community-blog/google-summer-code
https://www.concrete5.org/community/forums/chat/summer-of-code-ideas/#953339

One of the points to come from background discussion was for any asset recompile to be actioned on save, not on view. That way, site visitors don't get any slow page loads because the assets are always already compiled.

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Feb 15, 2019

@JohntheFish I'm not sure how relevant this is anymore as the concrete5 composer installation flavor ships with Laravel mix:
https://github.com/concrete5/composer/blob/acc4eced6d60cc5d92999543c40df42c62e349a2/package.json#L13

Laravel mix is basically just an extension / DSL to use Webpack. Webpack seems to be currently the de-facto for this kind of stuff (at least for the time being, although these tend to change every few years).

Even if a PHP based implementation were to be introduced, I would imagine the backend implementation would still use Node for compiling (as Webpack does). Although, it would be nice addition for people not familiar setting these tools up.

Another benefit would be to get rid of the "hardcoded" less dependency from the core.

Keeping all this in mind, I think the core team would need to put a bit of effort into consolidating the standards and suggested ways of implementation in the core. I know a lot has happened regarding this but 5.7+ have been out for a while and now we are still in a situation where we have:

  • At least 3 different ways of compiling the CSS assets (built in less compiler, plain CSS, Laravel mix)
  • At least 3 different ways of managing the database schema (old db.xml, new db.xml, Doctrine entities)
  • At least 3 different ways of defining HTTP routes (single pages, legacy routing layer, new routing layer)

This all causes a lot of unnecessary mess and breaking code in unexpected places. I was recently creating a script that syncs content between two separate concrete5 databases on the same server, which on the concept level should be a breeze to implement using two separate database connections. Because of some hard coded dependencies and the way the database entities are and are not used, made this a whole lot harder to implement than it would have to be. I would imaging copying entities using code should be only a few lines of code but it ended up being days of development work, debugging and writing very hacky looking code.

I hope with v9 we would see a situation where the core would be "cleaned up" a lot / consolidated regarding these factors and would ditch all the competing ways of implementing X to their own compatibility layer for those that need to support a legacy code base.

@mlocati

This comment has been minimized.

Copy link
Collaborator

mlocati commented Feb 18, 2019

At least 3 different ways of compiling the CSS assets (built in less compiler, plain CSS, Laravel mix)

Well, the built-in less compiler should be used only for theme customizing. In other cases you should use plain CSS.
Laravel Mix is used just to generate plain CSS, it's not strictly part of concrete5.

At least 3 different ways of managing the database schema (old db.xml, new db.xml, Doctrine entities)

You should never use the old db.xml format: the new format is much more powerful and easy to be used (having an XML schema definition, IDEs can suggest completion, and concrete5 can automatically check their validity).
Entities are totally different way of defining data structures: not everything is (and IMHO should be) an entity.

At least 3 different ways of defining HTTP routes (single pages, legacy routing layer, new routing layer)

With "legacy routing layer" are you referring to the old "tools" approacj? If so, I do agree that they should be removed (and I slowly remove them, see #7314 and #6951 for example).
With "single pages" are you referring to block and page methods which are automatically mapped to routes? If so, I think that they are really handy...

@ahukkanen

This comment has been minimized.

Copy link
Contributor Author

ahukkanen commented Feb 19, 2019

Entities are totally different way of defining data structures: not everything is (and IMHO should be) an entity.

I disagree with this as it just adds another way of doing things. Other option would be to get rid of the entities, obviously, which I personally don't like very intriguing.

With "legacy routing layer" are you referring to the old "tools" approacj?

No, I mean the config based routes which I believe will be replaced with the PHP based routing layer. Yeah, tools add the fourth option for doing the same thing.

Automated single page / block routing actions are in my opinion redundant.

In general I think the most beautiful architectures are those that clearly encourage a simple and a single way of doing things. I know this is not part of the c5 "cut'n'paste" philosophy but I'm just trying to raise awareness what this type of overlap could cause.

In the example script that I mentioned above, I needed ~30 lines of code to update an attribute with two simultaneous database connections.

@JohntheFish

This comment has been minimized.

Copy link
Contributor

JohntheFish commented Feb 19, 2019

Getting back to the original proposal, rather than philosophy of 10 different ways of defining data or routes.

Arguments given against proposals such as this appear contrary to arguments for proposals such as #7355 (and similarly contrary to existing capabilities such as short tag expansion)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment