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

Gulp stuff should be generalized, consolidated out to the root src folder, and documented #5450

Closed
DaRosenberg opened this issue Jun 28, 2015 · 30 comments
Assignees
Milestone

Comments

@DaRosenberg
Copy link
Member

Background

With VS2015 MS is embracing Grunt/Gulp as the automation tool of choice for client-side assets (styles, scripts, images etc). These tools are built on Node.js and are extremely powerful, flexible and versatile. They are commonly used for tasks such as compiling LESS/SCSS, bundling, minification, source map generation, browser vendor prefixing, etc etc.

True to this direction, Web Essentials for 2015 has dropped support for all these tasks, instead instructing developers to use Grunt/Gulp for this.

VS2015 has a new tool called Task Runner Explorer that allows you to execute your Grunt/Gulp based pipeline and integrate it with you VS workflow by binding Gulp tasks to project events (such as open, build, clean). This tool is also available for VS2013. There is also a tool called Grunt Launcher (slightly misnamed as it also supports Gulp) that adds support for installing all required node modules for your pipeline by right-clicking on your Packages.json file and choosing NPM Install Packages. This tool is available for both VS2015 and VS2013.

Mm and what does this have to do with Orchard?

To enable us to use VS2015, Orchard has already started using Gulp to perform these tasks in a few modules:

  • Orchard.Layouts
  • Orchard.DynamicForms
  • Orchard.Azure.MediaServices

Why these modules, you ask? Because these modules were already using Web Essentials to perform these tasks. These modules now all have a Packages.json file (that tells NPM which node modules are required) and a Gulpfile.js which contains the automation logic. (You could of course execute Gulpfile.js yourself using standard Node.js from the command line, but the tools mentioned above let you conveniently execute it from within VS instead, and have it be triggered by various VS events.) These module all have slightly different automation pipeline needs, and so the module dependencies and the Gulpfile.js logic differ between these modules.

Problems with current state

I believe we should not continue further on this path.

Having each module contain it's own complete automation pipeline has a number of issues:

  1. There is a massive amount of duplication.
  2. For every module where want to perform one or more of these tasks, we have to reinvent the wheel and create the whole pipeline and install all Node.js packages etc.
  3. Any improvements will have to be manually applied in each module.
  4. Installing the required Node.js modules (and all their dependencies) creates a massive directory structure under node_modules directly in the module directory. This can be .gitignored to prevent it from going into source control, but due to the way Orchard.Web is published for deployment, their presence on disk causes the publishing to fail.
  5. path is too long #5446

Proposed solution

I think the problems mentioned above could be mitigated if we generalize and centralize the concept out into the root src folder. In other words, create one Gulp automation pipeline that does this work for all modules in the code base.

Essentially I think we should:

  1. Create one Packages.json file in the solution root.
  2. Create one Gulpfile.js file in the solution root.
  3. Create and document a convention for modules to follow in terms of how to organize their client-side assets
  4. Configure that one Gulpfile.js to find client-side assets in all modules (according to that convention) and perform these tasks for all of them.
  5. Remove all Gulp and NPM stuff from individual modules.
  6. Obviously have both Packages.json and Gulpfile.js in source control, but .gitignore the node_modules folder.
  7. Hopefully Task Runner Explorer can integrate the same way with a solution-level Gulp pipeline - otherwise we'll have to figure out a way to overcome that.

This would solve all the issues I think.

Ideally the new Orchard.Resources module propesed in #5438 should also be able to utilize this unified pipeline.

Some general questions we should agree on:

  • What tasks do we want our general Gulp pipeline to perform? I have a rough idea, I will add it to the discussion later.
  • What should the convention/contract look like? I.e. how should modules organize their assets for inclusion in this pipeline, and how can modules have a say in how assets get treated? For example, how can modules specify groups of assets that should be bundled? Again, I have a rough idea, will add it to the discussion.
  • Do we keep the output in source control? Basically we can follow one of two principles:
    • The source assets are the only source code artifacts. The output .css and .js files are considered build output and not included in source control. This means you have to get the Gulp pipeline up and running to even build and run Orchard. If I think only selfishly I prefer this, because I will always have this up and running.
    • Both the source assets and the output .css and .js files are considered source code and both are included in source control. This means you don't have to bother with installing Node.js, installing the NPM packages and installing any required VS extensions unless you intend to make changes to these assets. If I think non-selfishly I prefer this, because it will significantly lower the barrier-of-entry and make it a lot easier to get Orchard up and running.

If we can agree on a framework for this, I think it would be good to go through all modules and restructure them to participate. Currently there is a lot of inconsistency across the code base, e.g. some modules have minified versions of some assets but not all etc. This would be a great opportunity to clean this up and get the benefits of bundling, minification, source maps etc. across the whole code base.

Which branch?

I am hesitant on where I think this change should go, 1.9.x or dev? I know it sounds huge and some of you will instinctively say "are you crayzee?? dev of course!" but before you make a conclusion, consider:

  • This change would not contain any functional change to Orchard itself (the output assets would still be the same)
  • This change would not break any existing modules - the new Gulp pipeline would be strictly opt-in
  • 1.9.x will still be active and around (probably) when VS2015 RTM hits and many people will want to use it

I am leaning towards kind of a hybrid:

  • Add the central Gulp stuff in 1.9.x but convert only Orchard.Layouts, Orchard.DynamicForms and Orchard.Azure.MediaServices to use it. Leave all other modules in 1.9.x alone. This will solve the build/publish problems people are seeing in 1.9.x.
  • Merge this to dev.
  • In a feature branch off dev, go through all other modules and update them to participate in the new pipeline.

Documentation

Finally I think it will be crucial to document this stuff. Both for explaining how it works, but also as guidelines for creating new modules that wish to participate in the client-side asset build pipeline. Once the questions have been sorted out and we agree on a general direction, I will gladly take it upon myself to document this.

@DaRosenberg DaRosenberg self-assigned this Jun 28, 2015
@DaRosenberg
Copy link
Member Author

OK, here are my 2 cents on the questions posed.

What tasks should the framework perform?

Based on the types of assets that are currently used by modules in Orchard, I believe a good starting point would be to have the following 4 pipelines provided and automated by the framework:

  • LESS (*.less)
    1. Compile down to CSS
    2. Hand off as input to the CSS pipeline
  • CSS (*.css)
    1. Bundle (concatenate)
    2. Autoprefix (add browser vendor prefixes)
      • At this point output to target .css file
      • Add sourcemap to bottom of file
    3. Minify
      • At this point output to target .min.css file
  • TypeScript (*.ts)
    1. Compile down to JavaScript
    2. Hand off as input to the JavaScript pipeline
  • JavaScript (*.js)
    1. Bundle (concatenate)
      • At this point output to target .js file
      • Add sourcemap to bottom of file
    2. Uglify (minify)
      • At this point output to target .min.js file

For each of these four pipelines, the framework would provide three tasks:

  • Build to be performed on solution build
  • Watch to trigger pipeline execution whenever one of the input files changes
  • Clean to be performed on solution clean

What is the convention/contract for modules to utilize the framework?

After giving this some thought I feel it would be very difficult to make this entirely conventions-based (particularly, it's hard to imagine a practical way for modules to specify bundling groups for included assets using a purely conventions-based approach).

Instead I propose a very simple JSON format through which a module can provide its desire to utilize the framework, which assets it wants processed, and how to group them into output files. To opt in to the framework, a module (or a theme for that matter) should have a file named Assets.json in its root folder. The Gulp script will scan modules/themes and look for this file and load it.

Here is a sample fictitious Assets.json file (I think most cases would be much simpler than this, but to illustrate the concepts):

[
    {
        "inputs": [
            "Assets/forms.css",
            "Assets/Bootstrap/bootstrap.less",
            "Assets/Bootstrap/theme.less"
        ],
        "output": "Styles/Styles.css"
    },
    {
        "inputs": [
            "Assets/forms-admin.css",
            "Assets/menu.dynamicforms-admin.css"
        ],
        "output": "Styles/Admin.css"
    },
    {
        "inputs": [
            "Assets/LayoutEditor/Directives/Fieldset.js",
            "Assets/LayoutEditor/Directives/Form.js",
            "Assets/LayoutEditor/Models/Fieldset.js",
            "Assets/LayoutEditor/Models/Form.js"
        ],
        "output": "Scripts/Scripts.js"
    }
]

Notes:

  • The JSON file is an array of groups where each group specifies a set of input assets and results in one output file.
  • A group can contain mixed types of input assets, as long as those can be processed into the same output file.
    • For example you can specify both LESS and CSS assets in a group targeted for a .css output
    • And you can specify both TypeScript and JavaScript assets in a group targeted for a .js output
    • But you can't mix and match - if you try, the build will throw an error
  • All file paths are relative to the module/theme root folder.
  • It would be convention to use Assets as a folder for the input assets and keep those separate from the output assets, but not required.
  • As a module/theme developer you need to (once) include the output files in your project and add them to source control.
  • The output files can either be included as files, or they can be declared in a resource manifest and required by yours or other modules.
  • If you don't put this file in the root of your module/theme, the Gulp framework will completely ignore your module

@bleroy
Copy link
Member

bleroy commented Jul 1, 2015

Thanks for writing this. I've had to solve some of the same problems on DecentCMS, because it's also modular, and also uses Grunt/Gulp/Npm (of course it does, it's Node). It's still work in progress, but I'll share some of what I've found.

First, on Gulp vs. Grunt, DecentCMS currently uses Grunt, but I think I'll move to Gulp eventually. I think it would be a bad idea to support both, and if you're going to choose one, Gulp is more modern and trends to become the standard.

While I've had to move some of the tasks to the "solution" level, I still have one package.json in each module (not just for Grunt: modules are real npm packages, so this is needed for other things). There are however top-level tasks that know how to drill down and execute the local tasks for each module. Under /lib and in the top gruntfile.js, you'll find some of the utilities I've written to achieve that.

Because I have both top-level and local tasks, with top-level ones typically calling the local ones, I can run at the level I choose, as needed. For example, I can run all the tests in the solution in one operation, or I can focus on the test task for one specific module if I need to.

The local gruntfile.js that you'll find in any module is typically a very simple boilerplate copy of the module-gruntfile.js that's at the top-level.

The problem of nested npm dependencies is also something I'm experiencing, so I'm very interested to see what kind of solution we come up with in Orchard. I've given it some thought, and my solution will probably involve leveraging some of the work from the npm team to flatten dependencies, coupled with some additional code in my own dependency injection code. But yes, lots of things will move to the top level, like you're suggesting.

Currently, the tasks that I'm handling are install and update (which recursevely npm installs or updates all module dependencies), test (recursively runs test suites in all modules), imagemin. I have local less, and scripts tasks where relevant, so those are not top-level, only local. This may change.

@bleroy
Copy link
Member

bleroy commented Jul 1, 2015

Another thing I'd suggest is to put that JSON into package.json: package.json is extensible, and I'm not sure it's a good idea to introduce a new file. Grunt & Gulp already use package.json for a number of things, and it seems like a good thing.

@sebastienros sebastienros added this to the dev milestone Jul 2, 2015
@bleroy
Copy link
Member

bleroy commented Jul 2, 2015

Another comment on checking-in built js and css: currently we offer two ways to install Orchard, the zip pre-compiled package, and the source code. If we want to keep it simple, the pre-built has no tooling, just the binaries, and that includes built css and js. It just works. Then the source code, which already isn't ready to run before you build it, would just have a couple more build steps. There probably is no point in being able to read the results of the TS compilation or the less process directly in the source tree. The only concern I'd have, as I've expressed before about TS, is that we're growing the list of pre-requisites for people who need to build without VS (that includes many continuous integration setups), but as Gulp is becoming more and more mainstream, this doesn't look like such a burden. At least the runtime list of dependencies is still just ASP.NET MVC.

@DaRosenberg
Copy link
Member Author

Thanks for the good input Bertrand!

I think in your case with DecentCMS it makes a lot of sense to retain a packages.json file in each module, given that they are themselves NPM packages. In our case with Orchard, I think it doesn't so much. But I'm interested in what you mean by Gulp already using Packages.json for a number of things - can you elaborate, or perhaps post some links to more info? A quick search on this didn't provide me with anything.

I'll have a look at what you built for drilling down into modules from the top level, as it sounds like you already solved a lot of what I would need to achieve.

Regarding having output assets in version control, I am very tempted to go the way you are suggesting (i.e. not having them in version control) as it just seems cleaner to me, and for me personally it would not be an issue. But I'm concerned that it'll cause trouble for others getting started with the source, specifically:

  1. You would no longer be able to build Orchard outside of Visual Studio (command line or CI builds etc) unless we also hook the Gulp stuff into MSBuild somehow. You have any experience or ideas on how to do that?
  2. Even in Visual Studio there is a manual step required to get it started, i.e. installing the NPM packages. This needs to be done for the Gulp integration tools to ever recognize Gulpfile.js and allow you to execute its tasks. And it's not very discoverable either, you basically have to just know that it needs to be done. If you don't there are no error messages, no nothing, just 404 when you eventually run the site and it tries to load the resources. Any suggestions on this?

@MpDzik
Copy link
Contributor

MpDzik commented Jul 3, 2015

I would suggest avoiding any solutions which require Visual Studio to build Orchard. I work in a lot of projects which rely on building Orchard from the console (for example on a build server) without VS. Additionally, I sometimes tend to work outside VS (for example in WebStorm or Sublime Text). Therefore, I think that forcing developers to use only VS to work with Orchard is a bad idea.

@IntranetFactory
Copy link

I would suggest avoiding any solutions which require to setup additional tools and dependencies. I always prefer to clone a repo, open the solution, hit F5 and it should work. So no additional tools, no nuget packages, no bower, no npm which is not included in the repo. I see nuget/npm/bower as a convenient way to update dependencies when needed. But I really hate being offline and recognizing that a dependency is missing or in the wrong version. So in our own repos we always commit all dependencies as space is cheap.

@DaRosenberg
Copy link
Member Author

Upon further pondering, I'm realizing I sounded the alarm bell for no reason.

You can of course always build outside of Visual Studio, because Gulp executes naturally from the command line. Visual Studio would never be required. We can either:

  1. Preferably, extend the Orchard.proj MSBuild file to install NPM packages and execute the Gulp tasks, OR
  2. If that's not possible for whatever reason, extend the ClickToBuild.cmd and ´ClickToBuildAzurePackage.cmd` batch files to do the same thing.

The only prerequisite would be having Node.js installed.

The issue I mentioned with discoverability inside Visual Studio however still applies. Any input on how to improve that context would be most welcome.

@Piedone
Copy link
Member

Piedone commented Jul 3, 2015

If we have the assets in the solution as we have now then I don't see why it would matter too much what build support there is: if you develop Orchard itself then you'll need some tools during development, otherwise in your own solution you're free to do whatever you want (and e.g. write plain CSS, or use Grunt/Gulp/whatever to generate it).

And I'd suggest to keep output files (CSS, JS) in the solution for the sake of simplicity. Can't really see any drawbacks: one pain point could be merge conflicts in generated files, but these are easy to resolve and only impact the development of Orchard (and not everyone developing with it). Storage (even if we talk about repositories) is a non-issue.

Long story short: whatever we do, it shouldn't affect people developing on Orchard, only people developing Orchard itself.

@sebastienros
Copy link
Member

My concerns:
1- With this solution, whenever a js/css file is touched then the full gulp pipeline would be restarted, right? As the gulp script wouldn't know which file is modified, and Visual Studio would register a filewatcher on every single asset file.
2- Are you sure the file length issue would be fixed by moving the node_npm folder at the top?
3- I would love to see it not integrated by default with visual studio if I don't want to use it, as today it's starting automatically for nothing, making the build really slow and breaking the command line.

I am worried that the change is huge here for what it adds. And right now the command line build is broken in 50% of the cases because of the file length limitation. A conservative change would be to revert the gulp integration to fix the build scripts, and in parallel work on the solution wide design you suggested. An even more conservative strategy would be to do it in 2.0 only.

@bleroy
Copy link
Member

bleroy commented Jul 21, 2015

  1. I don't think this is true: doesn't VS simply emit an event when you save a file, that the Gulp stuff can hook onto? Remember that that Gulp stuff is already used by pretty much the whole Node community. As far as I know, it works great without ruining perf.
  2. It would be mitigated, but it would still be a problem for modules that have a very deep dependency graph. That is, until the next version of npm, where they will flatten things as much as they can. By then, that concern will be completely gone except maybe in very very marginal cases.
  3. Yes, the watching tasks should be opt-in, but that may be an issue with how the feature is built in VS more than how we use it.

@DaRosenberg
Copy link
Member Author

All that @bleroy said, plus some comments:

  1. No, not true. Only the file that actually changed will be sent through the Gulp pipeline.
  2. For our scenario it would be fixed. The file length issue is only an issue when publishing, and if we move it all to the top level publishing will never see it, as publishing only happens using either Orchard.Web or Orchard.Azure.Web as roots.
  3. We can remove the event bindings if you prefer, and instead you'll need to invoke the "build" and "watch" tasks manually from Task Runner Explorer. However, depending on what you mean by "breaking the command line" I'm pretty sure that issue will go away with this refactoring.

I wouldn't call this change huge, but certainly it's significant. I think you're underestimating the value it will add though. I'm intending to get started on it fairly soon, but in the meantime I guess it wouldn't hurt to remove the event bindings if it's breaking things for you or slowing them down unacceptably.

sfmskywalker pushed a commit to IDeliverable/IDeliverable.Widgets that referenced this issue Jul 23, 2015
@sebastienros
Copy link
Member

Do I summarize all the goals with these points ?

  1. All assets should be in the solution in the end, even if they are updated by Gulp.
  2. If there are no changes of the source files, the process should not be started automatically by Visual Studio. And the process should not be started by a command line build, meaning that if I haven't changed any assets from Layout, I don't want any package to be downloaded, or all assets to be rebuilt.
  3. Each module contains a Gulp file to build its own assets. If a module asset is changed, only this Gulp file is running.
  4. If downloaded, the packages should not live inside each module by at the root, for instance in the /build folder.

@DaRosenberg
Copy link
Member Author

  1. Correct, output should remain in source control.
  2. Command line build - correct. Running the gulp pipeline should be considered an edit activity, not a build activity. As for starting it from VS, well... my vision was for it to be hooked into the VS events (open, build, clean) but if others think that's too intrusive, we can settle for running it manually.
  3. No, incorrect. There is only one gulp file, at the solution level, but each module contains a small "asset manifest" that is input to the global process. But correct that only changed assets will be processed, not everything at every change. The global gulp file will be smart enough to only trigger processing for items which are newer than their respective outputs.
  4. Correct.

I plan to work on this tomorrow.

@DaRosenberg
Copy link
Member Author

Implemented this today - I'm very happy with the result! It's fast, seamless, clean, unintrusive (you won't notice it unless you choose to edit and recompile client-side assets) and most importantly, it fixes the publishing issues we've been seeing.

Pushed it to 1.9.x as planned, and merged to dev so we can continue to apply it for all the other modules there over time.

The new solution-level Gulpfile.js and Package.json files are located here:

image

In Task Runner Explorer you'll find 3 tasks:

image

  • build does an incremental build (each asset group is built only if one or more inputs are newer than the output).
  • rebuild does a full rebuild (all assets groups are built regardless of timestamps).
  • watch starts a continuous watch (each asset group is built whenever one of its inputs changes).

As per @sebastienros's request, I did not bind the Gulp tasks to any Visual Studio events. You must manually invoke one of these three tasks from Task Runner Explorer, depending on what you intend to do.

Please try it out and let me know what you think!

Here's an example from Orchard.DynamicForms of what an Assets.json file might look like:

[
    {
        "inputs": [ "Assets/JavaScript/Lib/**/*.js" ],
        "output": "Scripts/Lib.js"
    },
    {
        "inputs": [ "Assets/JavaScript/LayoutEditor/**/*.js" ],
        "output": "Scripts/LayoutEditor.js"
    },
    {
        "inputs": [ "Assets/CSS/*.css" ],
        "output": "Styles/@.css"
    }
]

Note the Styles/@.css syntax in the last asset group. This means basically "don't perform any bundling, create one output file per input file, using this target path where @ is the base file name of the input asset file".

I will proceed to write up some documentation on how the thing works.

@Skrypt
Copy link
Contributor

Skrypt commented Jul 26, 2015

Works perfectly. I like it. 👍

@sebastienros
Copy link
Member

About the assets file, is it a standard format or a custom one?

@DaRosenberg
Copy link
Member Author

New standard, patent pending. ;)

@thekaveman
Copy link
Contributor

Just in case this isn't obvious to someone else (it wasn't for me)...

When you first load Orchard in VS 2015 and look at the new Task Runner Explorer, you'll see that it has located Gulpfile.js, but is not finding the tasks contained therein:

image

This is because you are missing the required NPM packages. Either use the command line, or install the Grunt Launcher Visual Studio extension, which allows for right-clicking on the Package.json file to install NPM packages. Afterwards, refresh Task Runner Explorer to see the tasks:

image

@DanielStolt I know you've mentioned this in various comments on this thread - I wanted to make it explicit with screenshots for others like me who can't seem to slow down and read the whole thing ;-)

@dcinzona
Copy link
Contributor

dcinzona commented Aug 5, 2015

That's odd, VS2015 automatically downloaded everything in Package.json for me on startup. I do not have the Grunt Launcher VS extension installed. It was all backed in to VS 2015

@thekaveman
Copy link
Contributor

Odd indeed. I opened and closed VS many times, cleaned and rebuilt the solution...nothing.

@dcinzona
Copy link
Contributor

dcinzona commented Aug 5, 2015

Is Package.json in the solution when you open it or is it just sitting in the src directory, but not included in the actual VS solution?
screen shot 2015-08-05 at 1 06 51 pm

@thekaveman
Copy link
Contributor

Yes, it was included in the solution when I opened.

@dcinzona
Copy link
Contributor

dcinzona commented Aug 5, 2015

Weird, I wonder if there are some extensions that are breaking it for you? I have this working on two completely separate computers and the Package.json file is automatically parsed successfully on both.

Not only that, but when I make changes to it, the npms are automatically downloaded.

@thekaveman
Copy link
Contributor

I am using the Community edition of VS 2015 - same for you, or do you have a full version?

@dcinzona
Copy link
Contributor

dcinzona commented Aug 5, 2015

I have VS 2015 Pro... I wonder if that's the issue...

On Aug 5, 2015, at 5:00 PM, Kegan Maher notifications@github.com wrote:

I am using the Community edition of VS 2015 - same for you, or do you have a full version?


Reply to this email directly or view it on GitHub.

@DaRosenberg
Copy link
Member Author

I have VS 2015 Enterprise and it does not automatically install the packages for me for this particular Package.json file (presumably because it's on the solution level) but it does so for any project level file. However, if I open the file and just save it (no changes) VS installs the packages. No need to install any extensions.

@rtpHarry
Copy link
Contributor

I guess you still have several months before this becomes an issue but I noticed today that Bootstrap 4 is moving from Less to SASS as its preprocessor. At the moment the pipeline doesn't have any support for it.

Seeing as assets.json is going to be promoted as the new standardised way for module and theme developers I think SASS support should probably be integrated into the process even if there aren't any official Orchard modules currently using that part of the pipeline.

@DaRosenberg
Copy link
Member Author

@rtpHarry I do agree. As mentioned in today's meeting, it was always my intention to have the assets pipeline to support both.

@mwpowellhtx
Copy link

This may have been addressed, I don't know. I have some boilerplate Gulpfile.js stuff, basically some tasks that apply across a suite of projects in my VS2015 solution. However, there are a couple of minor differences that I want to capture via project level configuration. However, it seems that the VS2015 tooling does not "see" the linked Gulpfile.js (i.e. I literally added the boilerplate as a link, with require for the configuration). Sorry is this is slightly tangential or off topic, but it seems relevant to the proposed question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests