OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Consolidate third-party client-side resources into dedicated module #5438

Closed DaRosenberg closed 8 years ago

DaRosenberg commented 9 years ago

We have discussed this in a few meetings, and I'd like to get started on this.

To recap, the idea is to take all of the third-party client-side libraries we have scattered across different modules and move them all into a module dedicated to just this purpose. We could reuse the Orchard.JQuery module but rename it to something more general like Orchard.Resources. It would include the jQuery resources of course, but also other things.

The new module would not contain anything except a bunch of third-party libraries in the form of LESS, CSS and JavaScript resources. It would contain a resource manifest and define resources for these libraries, so that other modules/themes can require them. The advantages would be less duplication, standardization on one version of these libraries across the codebase, and less problems with conflicts when multiple modules/themes try to include their own versions of these libraries.

The new module would ideally define these resources both on a granular level (individual components) and as well-defined larger packages grouping the smaller components for convenience. E.g. Bootstrap contains a bunch of smaller components that one should be able to require individually, but there should also be a big-ass Bootstrap resource defined by the module that gives you everything.

Things to include in the initial release of new Orchard.Resources module:

This is just a starting point, the ones I can think of OTOH - feel free to add to this list!

Personally I would also love to have one unified build pipeline using Gulp for all of these resources. In other words, we should add them in their "purest" form (LESS where available, CSS otherwise, non-minified versions) and let our build script do:

  1. LESS compilation
  2. Minification
  3. Bundling (where appropriate)
  4. Source mapping
  5. Vendor prefixing

I think we should do all this work in a feature branch off dev.

Thoughts?

MpDzik commented 9 years ago

Very good idea!

Jetski5822 commented 9 years ago

tagit.js is used in a few places I think.

Do you not think that where the vendor has provided a min file, we should use that?

IntranetFactory commented 9 years ago

I think that it's important to always have the non min version in the repo. This will ensure that you can fix issues even in the case the the original repo with that component becomes unavailable.

I think it was not really good idea to use Angular. I'd suggest to stay as close as possible to pure web standards and stay away from all hyped, esoteric technologies like Angular, CoffeeScript, TypeScript etc.

Any 3rd party dependency limits the choices and developer mind-share - I would even question if using JQuery is a good idea in 2015 - see http://youmightnotneedjquery.com/

So instead of having more and more dependencies added, I'd prefer to discuss which of them are really required.

Skrypt commented 9 years ago

This would make it easier afterward to build the Bootstrap admin theme. If we need to bundle some of those together then minifying them also would be better. Right now I've seen some places where we are not using the minified version of the CSS files "maybe contrib modules". So, yes, this is all good. :+1:

DaRosenberg commented 9 years ago

@Jetski5822 OK, adding tagit.js to the list then.

Regarding vender-provided minified version, I actually find it much more convenient to disregard them and do the minification myself. Couple of advantages:

infofromca commented 9 years ago

using Gulp, SASS IS awesome. can we also optionally adding and choosing Polymer,Angular, CoffeeScript, TypeScript

DaRosenberg commented 9 years ago

@infofromca Angular would be included as mentioned, since it's already required by Orchard.Layouts. As for CoffeeScript and TypeScript, those are not really libraries/resources so I'm not sure what you mean, or how you see them fitting into this discussion? They would be better suited for the work discussed in #5450.

sebastienros commented 9 years ago

jQuery to be deprecated but still present. Replace all current references by the new module.

dcinzona commented 9 years ago

I personally really like Chosen (http://harvesthq.github.io/chosen/) and use it in a few custom modules... not sure if you would want to include it and use it in some of Orchard's filter dropdowns to support multiple filter lookups, etc.

Should the default theme also be updated to use bootstrap, in this case (since the admin theme will be using BS, it might make sense). This would also alleviate the customizations required to the layouts module for BS themes. I'd be interested in a poll to see how many Orchard themes are based on Bootstrap or some other framework like Zurb Foundation or Semantic UI.

SzymonSel commented 9 years ago

Would love to see this coming as default. I already have stopped using the Orchard.jQuery module.

sfmskywalker commented 8 years ago

Poll:

A) It is better to assign individual libraries (jQuery, Bootstrap, Knockout, etc.) to individual features (Orchard.Resources.jQuery, Orchard.Resources.Bootstrap, Orchard.Resources.Knockout, etc.) B) It is better to assign all libraries to a single feature (Orchard.Resources).

DaRosenberg commented 8 years ago

I vote B - single feature, but of course separate resources.

IntranetFactory commented 8 years ago

I vote A - I would even prefer separated modules for each library

DaRosenberg commented 8 years ago

I'd like to clarify my vote and provide my reasoning. I think features are unnecessary for this purpose, because nothing gets loaded into memory or executed here anyway. We are talking client side resources.

The only difference between one feature and separate features for this module, is whether the resources are declared and made available for inclusion or not. And the only way a resource gets included is that some view does Require() on it - if you don't use the resource, it will never get loaded, even if they are all in one feature.

The price: having to constantly keep your module or theme manifests (and perhaps recipes) up-to-date with whatever resources you happen to need. The benefit: zero.

Contrast this with features that contain actual services and code. In these cases features actually provide an optimization; they reduce the memory footprint, the attack surface area, optimize performance etc.

IntranetFactory commented 8 years ago

The only difference between one feature and separate features for this module,

How can I then e.g. disable one build-in library (let's say jQuery) to replace it with a different version provided by a custom module?

sfmskywalker commented 8 years ago

You could use the OrchardSuppressDependency attribute to suppress the jQuery resource manifest provider class. Even though there would be a single feature, we could still provide separate resource manifest provider classes.

sfmskywalker commented 8 years ago

(which I did in the PR)

IntranetFactory commented 8 years ago

So we have at least a second difference. I really don't understand why Orchard should become more and more monolithic. This might be nice if you're using everything only OOTB. But if you're using Orchard just as a part of your foundation then throwing all scripts into one big basket doesn't make sense.

DaRosenberg commented 8 years ago

Yes, I suppose that's another difference.

I should point out that I don't want to make Orchard monolithic, but neither do I want to overcomplicate it when there's no benefit. I simply think there's a balance to be stricken between simplicity/convenience on the one hand, and modularity on the other, and I think it this case the former outweighs the latter, because the modularity is more or less achieved by the separation into resources, and the additional separation into features provides no additional benefit.

As Orchard developers I also think it's our responsibility to carefully consider that balance, and not just fall on one side or another out of habit or rigidity, especially in cases where there is no observable benefit. I don't believe we are "throwing all scripts into one big basket" - they are separate resources, and each script will only come into play if explicitly referenced by a view.

IntranetFactory commented 8 years ago

With that approach you make a bunch of scripts available I don't want to allow my developers to use. So we need to communicate that, we need ensure in code reviews that nobody relies on them in our modules. That's obviously no problem for "Orchard developers", but for us that's probably the final disconnect.

But I understand that Orchard becomes more and more just another monolithic CMS and won't evolve as the universal, modular web platform which attracted us initially.

Piedone commented 8 years ago

My gut would go with A but I see no practical advantage either, so B, with one addition: resource names are in the global namespace, so they should be created with keeping this in mind. However I see no real danger here either: if the resource name includes the library name (like "jQuery", "jQueryUI_Core") then it should be unique enough. That is, unless you add a module also containing the same library: in this case one of the resources will "win".

@IntranetFactory if you don't want your developers to use an Orchard feature then you need to communicate that internally, regardless of what you need to to in order to user that feature. In this case it would be a Script.Require(), otherwise also a feature dependency. Why it the latter better?

IntranetFactory commented 8 years ago

@Piedone being able to remove unwanted modules completely is the reason I'd prefer to have client side libraries in separate modules. I'd prefer to remove e.g. Angular and Underscore completely. So moving all client side libraries into one module is the primary issue.

We automatically bundle all resources - so if Angular won't be at least a feature it will be in the bundle.

Piedone commented 8 years ago

@IntranetFactory I see, so having separate features even wouldn't be enough for you.

Regarding the module removal: what would be the advantage (apart from the minuscule saving on storage and deployment time) for you to have such resources in their separate modules?

And with all due respect, if you bundle all static resources, regardless of them being actually used or not but merely being present in the solution, then that's probably not the best way to do it any way. Just on a side note.

IntranetFactory commented 8 years ago

We bundle the scripts for the enabled modules and in our case that's exactly the scripts we'll need on the client.

We have each module in it's own repo - for various reasons:

  1. we need to be able to answer questions like - what was changed in Orchard.Import in the last 12 month. We never found a reliable way to get that answered from the Orchard commit history.
  2. we have different permissions for different developers. Most are only able to change the modules they are working on.
  3. we removed all Orchard features we don't use, code we don't have is code we don't need to maintain
  4. that's a common approach if you look at tools like bower or npm

So the monolithic repo structure of Orchard is the reason we're unable to contribute back. Seeing that Orchard should become even more monolithic makes it obvious that we're probably finally disconnected.

CSurieux commented 8 years ago

I also prefer separate modules in order to update version according each framework progression.

DaRosenberg commented 8 years ago

Some of the arguments for separate modules are valid points, but that wasn't one of the poll options - OP asked about separate features. Consolidation into one module is kind of the point of this whole work item, and separate modules would basically reverse all the work.

@CSurieux This point I don't quite understand. We are talking about modules that would be part of Orchard core, and the core distribution is versioned as a whole. One possibility is of course to break Orchard down into smaller, independently versioned, packages (like ASP.NET have done) - @Jetski5822 indeed has some ideas about that for Orchard2 - but that's a much bigger discussion and far out of scope for the choice OP is facing.

IntranetFactory commented 8 years ago

Consolidation into one module is kind of the point of this whole work item

The whole work item goes into the wrong direction. Angular doesn't belong into core. Not everybody uses Orchard as a monolithic CMS.

From my point of view the question of OP was to limited. It should have been

A) It is better to assign individual libraries (jQuery, Bootstrap, Knockout, etc.) to individual features (Orchard.Resources.jQuery, Orchard.Resources.Bootstrap, Orchard.Resources.Knockout, etc.) B) It is better to assign all libraries to a single feature (Orchard.Resources). C) It is better to have a dedicated modules for each library.

CSurieux commented 8 years ago

@DanielStolt how long have we already suffered from an old JQuery version in Orchard blocking some of our modules. Or a new JQuery module in Orchard with new JQuery version making obsolete lot of existing code. Ideally we should be able to benefit from 'side by side' version of js frameworks...but not all these frameworks support this concept (I like this idea in new .net). It would be nice to have an old module using JQuery 1.7 and the other stuff the last version.

For this reason I prefer to have my bootstrap version in my bootstrap module with my dedicated version.

So I prefer separates modules with possibility to switch them and replace by dedicated stuff.

DaRosenberg commented 8 years ago

@CSurieux Nothing is stopping you from doing so. Just create your own Bootstrap module, declare a new resource yourself, and have your views include that one. Or if you want, declare the same resource name and add a dependency on Orchard.Resources to have your resource override the core one. Seems to me you have full flexibility. Anyway, I don't see how this pertains to the discussion, since we are talking about modules included in the core distribution? All of the problems you describe would be exactly the same regardless of whether we separate resources into separate Orchard modules or not.

CSurieux commented 8 years ago

Nothing is stopping me for voting for C in A) It is better to assign individual libraries (jQuery, Bootstrap, Knockout, etc.) to individual features (Orchard.Resources.jQuery, Orchard.Resources.Bootstrap, Orchard.Resources.Knockout, etc.) B) It is better to assign all libraries to a single feature (Orchard.Resources). C) It is better to have a dedicated modules for each library.

DaRosenberg commented 8 years ago

@CSurieux Except the fact that those are not the voting options. And besides, even if option C was on the table, how exactly would it address the versioning concerns you described above?

CSurieux commented 8 years ago

It should be added. The versioning is not possible as I stated, but, as an example and again, again, being able to disableThe Orchard Module for Bootstrap is the solution I prefer.

IntranetFactory commented 8 years ago

those are not the voting options

Interesting - so options you personally don't like are not eligible?

CSurieux commented 8 years ago

It's a recent tendency here

DaRosenberg commented 8 years ago

No it's not about what I personally don't like, it's about the context of this work item and the set of options @sfmskywalker originally presented when he asked for a vote. The purpose of this whole work item is to consolidate resources into one module. It was discussed in a community meeting, and agreed upon by the steering committee members. I don't recall either one of you raising your voice in objection then, and it's a little late now that @sfmskywalker has already done all the work. Arguing to move in a completely different direction is non-constructive in the context of this thread, and only serves to pollute the discussion IMO.

DaRosenberg commented 8 years ago

@IntranetFactory You are not being very constructive now, bordering on offensive in fact, I'd ask you to please not let this discussion deteriorate into some sort of personal vendetta. As you can surely see, the vast majority of commenters on this work item support it.

CSurieux commented 8 years ago

@IntranetFactory I agree The reasons are not clear: no real turn over in the instances, request for result from MS, raise of some 'military thinking' actve faction, lack of global thinking on the users/targets in favor of a poor technical only concept, no vision of what could be a community ???

sfmskywalker commented 8 years ago

@IntranetFactory

With that approach you make a bunch of scripts available I don't want to allow my developers to use. So we need to communicate that, we need ensure in code reviews that nobody relies on them in our modules. That's obviously no problem for "Orchard developers", but for us that's probably the final disconnect.

Ok, but don't you have this challenge today as well? Since your developers could include any script provided by any module already.

The goal of this effort is to de-duplicate client libraries by moving them into a single module. Same principle as with the jQuery module. The Orchard.jQuery library was brought to life for a reason: being a resource provider for modules that need jQuery without the need to maintain their own version of the JS files. As it turns out, jQuery is not enough and we have plenty more scripts that we need in multiple modules.

But you understand the issue, and you offered an alternative option C: each library in its own module. Although that would work of course, there are at least two concerns with that approach that I can see:

  1. There already are a lot of modules, and this approach would easily add 10 or more modules to the solution with no advantages at all when compared to having a single module providing these same resources (if you do see more pros than cons, can you list them?).
  2. Each library per module means a library per feature. As explained by @DanielStolt: The price: having to constantly keep your module or theme manifests (and perhaps recipes) up-to-date with whatever resources you happen to need. The benefit: zero. And I agree. Having updated a bunch of modules to take on a dependency on the newly introduced features just feels unnecessary and silly.

From my point of view the question of OP was to limited.

Any suggestions are welcome of course. However, to @DanielStolt 's point, this ticket was created based on a public meeting, and the course of action had already been discussed and decided upon. I personally don't think it's a good option at all as I tried to explain above, but perhaps others are of a different mind like yourself. If we would do it as separate modules, you would have no issue having 10 or more additional modules in the core distribution?

sfmskywalker commented 8 years ago

@CSurieux

how long have we already suffered from an old JQuery version in Orchard blocking some of our modules. Or a new JQuery module in Orchard with new JQuery version making obsolete lot of existing code.

Interesting, but off-topic.

Nothing is stopping me for voting for C in A) It is better to assign individual libraries (jQuery, Bootstrap, Knockout, etc.) to individual features (Orchard.Resources.jQuery, Orchard.Resources.Bootstrap, Orchard.Resources.Knockout, etc.) B) It is better to assign all libraries to a single feature (Orchard.Resources). C) It is better to have a dedicated modules for each library.

So you favor a script library per module as well. I'd be interested to hear the pros and cons from you as well. The pros I see with A) are:

(the cons I see with C I already listed) Let me know.

It's a recent tendency here The reasons are not clear: no real turn over in the instances, request for result from MS, raise of some 'military thinking' actve faction, lack of global thinking on the users/targets in favor of a poor technical only concept, no vision of what could be a community ???

Can you please mind the trolling? Thanks

bleroy commented 8 years ago

It's the whole point of resource manifests to allow modules to rely on a single central repository of libraries, and avoid duplication and running side-by-side versions of libraries when we desire to avoid them (which is most of the time). It's been mentioned that you can still include a specific script, and run side-by-side if that's what you want (but shouldn't). You can also maintain specific modules for individual libraries if you want to. In Core, we prefer to keep the number of modules smaller if we can, because there is overhead in modules, and because technically, separate modules per library offer no benefit in versioning and updating, and keeping up-to-date. We're all about listening to suggestions from everyone in the community, but it doesn't mean that we should loosen our critical thinking. When a proposed idea is understood but isn't good or aligned with design principles at work in Orchard, it's our responsibility to the community to reject them, with an explanation for the decision.

IntranetFactory commented 8 years ago

@sfmskywalker @bleroy Thank you for providing detailed feedback. I'm probably the only one having problems with weekly meetings without any known, upfront agenda (at least I never found one) happening late in the evening. In 2015 there are better options to collaborate across organizations and timezone boundries. I really love the architecture of Orchard, but I'm not happy with the way the project is managed. I know that an issue is not the right place to discuss that - and I'm not aware about the right place so I'm going to not disturb here any longer.

bleroy commented 8 years ago

Forums or the weekly meeting are both good places for governance discussions, and collaboration process suggestions. Election time (which was this month) is also a good opportunity to air grievances and actively change leadership.

sfmskywalker commented 8 years ago

No worries. We actually don't have upfront agenda's, but a general structure that we follow (status, demos and any topics that people raise). The meetings take place every Tuesday 12 PM PST and is open to anyone and everyone (http://orchardproject.net/meeting). If you are not happy with the way the project is managed, attending the meeting is a good place where you can voice your thoughts, as well as the Orchard forums at CodePlex. The project is managed by a steering committee whose members are elected once a year, so that's another opportunity to influence the management of the project.

CSurieux commented 8 years ago

@sfmskywalker I see that it always ends back to the same attitude from comittee members: people not sharing the opinion of the comittee get accused of trolling or even worse.

Let me suggest 2 things:

sfmskywalker commented 8 years ago

@CSurieux Nope, on the contrary: I welcome other opinions, even ask for them. In fact, the very reason I started this poll in the first place is to get input from others to lower the risk of 'single thought errors' as you put it. You should try it sometimes. Also, I value opinions that are backed with solid reasoning and are on-topic. Ranting and trolling, not so much.