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

Convert existing core/default tasks to Plugins. #193

Open
jayharris opened this issue Jan 18, 2014 · 21 comments
Open

Convert existing core/default tasks to Plugins. #193

jayharris opened this issue Jan 18, 2014 · 21 comments

Comments

@jayharris
Copy link
Member

Lineman is big. 58MB is a lot, especially considering that many projects won't use all of the default modules. grunt-contrib-less and grunt-contrib-sass are both pulled in to the default project, even though it is highly unlikely that both will be used at the same time. The same also holds true for grunt-contrib-handlebars if the developer is using underscore, though underscore will always be loaded, since it is pulled in with Grunt.

The size of Lineman doesn't matter much locally, but can be troublesome when deploying, such as Git Deploy on Heroku or Azure where we have very limited space on the server.

I propose converting these default tasks to Lineman Plugins, rather than core tasks. And rather than referencing the plugins as Lineman Dependencies, put them in the Archetype and trigger an npm install on lineman new. This way, they could still be included by default, but developers could easily remove unused plugins through npm uninstall, rather than removeTasks.

Also, removeTasks doesn't lighten the dependency load (and disk size), but npm uninstall would. This model would be much less error prone, as there were already a few issues noted involving removeTasks (Such as #127, #135).

@Foxandxss
Copy link
Contributor

My opinion here is that those plugins are not that special. I mean, there are some plugins that are modified to be used with lineman, but others are just a plugin that you download a make a minimum configuration.

Maybe those plugins are no that good candidate for a lineman plugin but something you can npm install any time like any other plugin.

@jayharris
Copy link
Member Author

I think it is important to still include many / most of these existing ones by default (again, via the Archetype, so that they can be easily removed). It is be helpful for developers that are new to Lineman to get some base configuration with their first Lineman project. To me, there is more value to the Lineman ecosystem by including these by default and having experience developers remove them, then there is to start with a skeleton Lineman project and npm install only what you want / need.

@Foxandxss
Copy link
Contributor

Yeah, the idea is good but on the other hand, we will end with a lot of lineman wrappers around common tasks. It is just more work to maintain :P

@searls
Copy link
Member

searls commented Jan 18, 2014

I think this is a necessary and valuable discussion to have. Initial thoughts:

  1. I don't think it'd be wise to rip out each and every task module just to say we did it, because having dozens of oft-used plugins presents plenty of the problems (like compatibility).  I don't think anyone is arguing that however. 
  2. Let's draw up the size of each task plugin right now so we know the worst offenders and attack them first
  3. I definitely like the idea of the archetype including a few plugins locally by default

On Sat, Jan 18, 2014 at 2:31 PM, Foxandxss notifications@github.com
wrote:

Yeah, the idea is good but on the other hand, we will end with a lot of lineman wrappers around common tasks. It is just more work to maintain :P

Reply to this email directly or view it on GitHub:
#193 (comment)

@jayharris
Copy link
Member Author

We've already done the work. We already maintain the code. For each of the core Grunt tasks, we already have a corresponding Plugin file, so they already are plugins, they are just included in core repository. Extracting them would usually be little more than extracting the plugin file and Package dependency to a separate repository, plus some cleanup in tasks/load-stuff.coffee (which is probably an advantage).

I think this change might actually improve compatibility and maintainability. With the Package Resolution changes from last night, we no longer have a hard dependency on pkg.main. Because of that, we can update a lot of the Grunt tasks (grunt-contrib-less@0.7.0 is getting a bit old) whereas we were blocked before because the Contrib team removed main from every contrib package. However, with everything being included in Core on a fixed dependency version, rather than pulled in through the Archetype, we have to release Core every time we want to update one of these Grunt tasks.

Lineman Core shouldn't care which version of Less I use, or if I use it at all; we should not have to bump Core's version just to update contrib-less; and, I should be able to update Less without also being forced to pull in an updated version of Handlebars, CoffeeScript, and Sass.

@searls
Copy link
Member

searls commented Jan 18, 2014

Actually Jay, I don't think that argument is valid, because local installs already trump those bundled with core. So today if you want a newer (or older) version of grunt-contrib-less for your lineman project, you need only install it into your project. 

The more I think about this the more cautious I want to be. If a future lineman change requires all or most plugins to be updated, we both have to do a lot of work in updating them all and users have to bear a fairly heavy burden of making sure they get the compatible versions right. We get a little help from peerDependency locking, but not enough since we'll generally start out optimistically. 

In general I would probably not be eager to pull out anything that's both necessary for a vanilla lineman app experience and also unlikely to be swapped out in very high proportions. If it's not both those things I'd prefer to just keep it in core. That's my sense right now. 

One funny slash hilarious point that's actually salient at this moment is that lineman plugins can require other lineman plugins. We don't do a lot of this now, but I designed the plugin system to allow arbitrarily nested plugin resolution. That means lineman-vanilla could include a mix of six other lineman plugins if we so choose. 

If the versioning combinatorics above don't frighten you into a sense of some caution about pushing a ton of modules, perhaps that will :-P

On Sat, Jan 18, 2014 at 3:20 PM, Jay Harris notifications@github.com
wrote:

We've already done the work. We already maintain the code. For each of the core Grunt tasks, we already have a corresponding Plugin file, so they already are plugins, they are just included in core repository. Extracting them would usually be little more than extracting the plugin file and Package dependency to a separate repository, plus some cleanup in tasks/load-stuff.coffee (which is probably an advantage).
I think this change might actually improve compatibility and maintainability. With the Package Resolution changes from last night, we no longer have a hard dependency on pkg.main. Because of that, we can update a lot of the Grunt tasks (grunt-contrib-less@0.7.0 is getting a bit old) whereas we were blocked before because the Contrib team removed main from every contrib package. However, with everything being included in Core on a fixed dependency version, rather than pulled in through the Archetype, we have to release Core every time we want to update one of these Grunt tasks.

Lineman Core shouldn't care which version of Less I use, or if I use it at all; we should not have to bump Core's version just to update contrib-less; and, I should be able to update Less without also being forced to pull in an updated version of Handlebars, CoffeeScript, and Sass.

Reply to this email directly or view it on GitHub:
#193 (comment)

@jayharris
Copy link
Member Author

Out of curiosity (and for my own education), why would core care about a Grunt task version? And when I say core in this sense, I am excluding the Plugins folder.

I get and agree that the Plugin file would need to be tied to a direct version of the corresponding Grunt task. In this sense, there should be a specific and fixed version of a Grunt task within any Lineman plugin, core or otherwise.

But outside of the Plugin folder, where and why would core care? What could cause compatibility issues?

@jayharris
Copy link
Member Author

Thinking aloud, in theory this would also enable replacing core plugins with entirely different plugins. For example, I could uninstall the Watch task and replace it with another Lineman Watch plugin that used @davemo's watch system. Or using grunt-contrib-compass instead of grunt-contrib-sass.

@searls
Copy link
Member

searls commented Jan 18, 2014

@jayharris the reason why lineman locks to exact versions of everything it depends on is because chaos happens when you don't and those packages upgrade/change anything out from under you (there's no cultural equivalent to Gemfile.lock in the Node world. People don't use npm-shrinkwrap in practice and when they do, it's onerous to update things). Any of those tasks could update with breaking changes and--even though Lineman didn't change--new user projects may entirely fail to work.

In fact, not only is that a risk, but on multiple occasions we've been burned when loose version specifiers in the task modules themselves leads to transitive dependencies breaking and in return breaking all subsequent projects that run npm install after that update. We still live with that risk, but also have some scars to prove it (like our own fork of watch_r, for instance).

In general, I want to do the best I can to ensure that Lineman Just Works™ for folks, and locking down to exact known-working versions of dependencies is how we do that. Of course, that means we ought to be diligent about testing and updating those dependencies, and lately we have not been. That's why I think this issue is a good discussion, because if we could slim those down, that'd make that job a bit easier.

@jayharris
Copy link
Member Author

If the decision is to keep them bundled, that's cool. Unfortunately, I'm struggling to get around taking some of the bundled components completely out of the system and replacing them with alternatives without altering the core codebase. It's keeping us from migrating a few of our projects over to Lineman.

@davemo : How did you get around this with your custom Watch code under Lineman? Is it a plugin? Is it a custom Grunt task? Did you have to modify Lineman and/or fork the core code?

@davemo
Copy link
Member

davemo commented Jan 20, 2014

It's a custom grunt task that lives in ./tasks/watcher.js, just loaded it with linemans loadNpmTasks.

@searls
Copy link
Member

searls commented Jan 20, 2014

Jay, I'd suggest we should probably tackle any challenges you've had making overrides work separately. There's a good chance Dave and I are familiar with ways of overriding default behavior that you don't know yet. I suspect this because after all this time toodling around with lineman overrides, I've never felt like I needed to change core lineman to do what I want in a client project. 

Could you ping us with an example of something that was difficult to override?

On Mon, Jan 20, 2014 at 9:25 AM, Jay Harris notifications@github.com
wrote:

If the decision is to keep them bundled, that's cool. Unfortunately, I'm struggling to get around taking some of the bundled components completely out of the system and replacing them with alternatives without altering the core codebase. It's keeping us from migrating a few of our projects over to Lineman.

@davemo : How did you get around this with your custom Watch code under Lineman? Is it a plugin? Is it a custom Grunt task? Did you have to modify Lineman and/or fork the core code?

Reply to this email directly or view it on GitHub:
#193 (comment)

@jayharris
Copy link
Member Author

I did a bit of initial exercise on this, since this keeps coming up in other issues, and as discussed in #229 with the possible extraction of LESS and SASS and implementing Stylus as Core.

To start with, I extracted Coffee and LESS. This work is found in the Extract-Plugins branch. These were extracted to lineman-coffee and lineman-less.

The two Plugins are now loaded via the Archetype, rather than in Core. This is functional in the Branch. (The Archetype references the GitHub Repos for each plugin, since they have not been published to NPM.) With this idea, a developer can drop LESS support simply via npm uninstall lineman-less -S, and instead opt to use lineman-sass, lineman-stylus, or straight CSS development.

To test it out, Uninstall Lineman, and reinstall Lineman from the extract-plugins branch.

npm uninstall lineman -g
npm install https://github.com/linemanjs/lineman/tarball/extract-plugins -g

Then make a new Lineman project. Be sure to use the --install flag, so that npm install will be executed within the archetype.

lineman new bananas --install

One more note:
I moved lineman, lineman-coffee, and lineman-less to be Dependencies of the project, rather than DevDependencies. This was a conscious choice. The thought was, dependencies should be anything that is required for a production deployment, which all three are, and devDependencies should be anything that is only required for local development. In other words, anything for lineman build would be a dependency; anything that's just for lineman run would be a devDependency. Heroku, Azure, etc would need lineman, lineman-coffee, and lineman-less to build and deploy on the server, but once we fully converted everything to external plugins, things like lineman-jshint and lineman-coffeelint--and if we went that far, even lineman-spec or lineman-testem--would just be devDependencies.

@searls
Copy link
Member

searls commented Mar 15, 2014

Cool deal Jay. I don't have time to look at this closely but I still have some worries about anything we do complicating the end-user experience. Please hold off before pushing anything to master or npm until it has plenty of time in the oven. 

I personally disagree that any lineman stuff should be moved out of devDependencies. Grunt and gulp can just as well be used for deployment, but those teams violently agree they should be devDependencies. Moreover, I think this would meet opposition from most experienced Node users and without any material benefit. Servers performing the deployment will always need devDependencies (whether it's to run build tasks or tests). 

thanks for pushing it forward!

On Sat, Mar 15, 2014 at 11:44 AM, Jay Harris notifications@github.com
wrote:

I did a bit of initial exercise on this, since this keeps coming up in other issues, and as discussed in #229 with the possible extraction of LESS and SASS and implementing Stylus as Core.
To start with, I extracted Coffee and LESS. This work is found in the Extract-Plugins branch. These were extracted to lineman-coffee and lineman-less.
The two Plugins are now loaded via the Archetype, rather than in Core. This is functional in the Branch. (The Archetype references the GitHub Repos for each plugin, since they have not been published to NPM.) With this idea, a developer can drop LESS support simply via npm uninstall lineman-less -S, and instead opt to use lineman-sass, lineman-stylus, or straight CSS development.
To test it out, Uninstall Lineman, and reinstall Lineman from the extract-plugins branch.

npm uninstall lineman -g
npm install https://github.com/linemanjs/lineman/tarball/extract-plugins -g

Then make a new Lineman project. Be sure to use the --install flag, so that npm install will be executed within the archetype.

lineman new bananas --install

One more note:

I moved lineman, lineman-coffee, and lineman-less to be Dependencies of the project, rather than DevDependencies. This was a conscious choice. The thought was, dependencies should be anything that is required for a production deployment, which all three are, and devDependencies should be anything that is only required for local development. In other words, anything for lineman build would be a dependency; anything that's just for lineman run would be a devDependency. Heroku, Azure, etc would need lineman, lineman-coffee, and lineman-less to build and deploy on the server, but once we fully converted everything to external plugins, things like lineman-jshint and lineman-coffeelint--and if we went that far, even lineman-spec or lineman-testem--would just be devDependencies.

Reply to this email directly or view it on GitHub:
#193 (comment)

@jayharris
Copy link
Member Author

One common concern (Especially from @searls and @davemo) is that this would require lineman to be installed to the local node_modules.

It does not. I have disproven this theory in local testing.

lineman works fine if it is just installed globally. This is why we haven't needed it in local node_modules so far. It still wouldn't need to be, even under this new model. But it gets installed locally because (1) the Archetype references it in its package.json and (1) the plugins reference it as a peerDependency. But neither are necessary. If we remove both--dropping Lineman from the Archetype package.json and dropping it as a peerDependency from the plugin--then it won't get installed to the local node_modules yet it will all still work just fine. The plugins will work just fine. Everything will work just fine.

The plugins never actually even reference Lineman beyond package.json. So they don't need it! The only extra overhead to the disk would be that the plugins are in the local node_modules, whereas they were in the global space from Core, before.

@jayharris
Copy link
Member Author

@searls With Grunt and Gulp, it's not unheard of to deploy the root project. package.json can be mixed in with normal production dependencies that would need to be in wwwroot/node_modules, for example.

If we ever deployed the root for a Lineman project, I would completely agree with you that they should be devDependencies. However, because we are only ever deploying dist, I disagree. I think Lineman's model is unique enough from Grunt / Gulp where this rule doesn't apply in the same fashion.

There's nothing stopping anyone from having an app/package.json, which would contain the traditional production dependencies, and an npm install from within dist after Lineman has done it's thing.

@searls
Copy link
Member

searls commented Mar 15, 2014

@jayharris that's a valid point, but I still don't see a compelling reason to put a development tool into the main dependencies hash. Anything doing a build-and-perform action will always need the entire set of dependencies to work, and I definitely don't want to send anyone the message that Lineman runs at production-time. A huge number of people see that Lineman ships with an express server for dev-time stubbing and immediately jump to the conclusion that Lineman is an app server to be deployed; I don't want to do anything to perpetuate that idea.

@searls
Copy link
Member

searls commented Mar 15, 2014

@jayharris I don't think removing all references to lineman from the things that depend on it (projects and plugins) is tenable, certainly not if the only benefit is to speed up project creation time. As the number of plugins grow, and as lineman itself continues to change, I don't want to force users (and plugin authors) to opt-in to specifying to a lineman version range when a BC-breaking change is introduced (either into lineman, between lineman and a plugin, or between two plugins).

Version-locking culture is awful enough in Node that plenty of projects just stop working arbitrarily after a certain calendar date, and I don't want to exacerbate that for Lineman projects just to speed up build time.

I view this conversation of pluginify-ing Lineman's defaults as an admission that lineman core should become so-tiny-as-to-eliminate-the-cost of that initial install time anyway.

@searls
Copy link
Member

searls commented Mar 15, 2014

Jay's experimenting brings up another point: it looks like this heads us in a direction where a new lineman project will be created as an ala carte selection of plugins at project-creation time.

If we're going to let users pick their new-project-time plugin installs (and I think we should if this is going to be of any value to end-users), then I think we should have a fantastic, instructive, and discoverable user interface for users to pick the plugins that are right for them.

Yeoman and Grunt-init use interactive CLI apps (which is obnoxious when you go through the wizard frequently). We could do that, or we could come up with clever CLI options (which are hard for many users and not particularly discoverable).

I think I prefer the idea of having meta-plugins that represent recipes of other plugins. Because of the recursive search Lineman does when loading plugins, you could easily have a lineman-recipe-vanilla on npm that does nothing but list lineman-less and lineman-coffee in its package.json, and it would install and enable both plugins. The lineman new command could then be as simple as defaulting to vanilla but also offering a list of lineman-recipe-* projects in npm with vanilla as a default. That way the archetype could remain incredibly simple.

One issue with my idea above that the example code would probably need to either be simplified or go away (short of us building an example-generator tool at install-time, which sounds way too clever for now).

@davemo
Copy link
Member

davemo commented Mar 15, 2014

I skimmed a bunch, but I like the idea of recipes and keeping the project archetype simpler.

@jasonkarns
Copy link
Member

I much prefer this idea that 'recipes' would be meta plugins that would be
installable, as opposed to the current template approach. It feels weird to
use a template to start a new project because it means git-clone and thus
inheriting into the git history of my project the commits of some 3rd party
tool developers.​

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

No branches or pull requests

5 participants