[Proposal] Federate core hooks

[mikermcneil]

Federate core hooks

This is the first pass at a proposal for how we could go about implementing custom builds of Sails. This is something we've been working towards for a long time, and it will likely not make it into the v0.12 release. But it is something that we plan to implement this year.

1. Installing core hooks

Goal: to provide a lighter customizable install of Sails.

Currently, while any/all hooks can be disabled in Sails, all core hooks and their dependencies are still bundled as part of Sails core. This is ideal for the basic default install, but annoying when you are running npm install locally in a Sails app and you aren't using half of the stuff you're waiting to install. Plus, if you aren't using Socket.io-powered features, you shouldn't have to wait for the native dependencies of sails-hook-sockets to compile (and furthermore, you shouldn't have to worry about ensuring your server is node-gyp-ready).

I see three major solutions to solve this:

(A) Do it like Grunt

We could have sails-generate-new add the appropriate versions of hooks as dependencies in new Sails apps' package.json files. This is great because it completely separates hooks from Sails core, but the major downside is that now there are a whole bunch of new version strings to keep track of in your application. This is very similar to what happens with grunt-contrib-* dependencies, and much like a Grunt plugin is pretty much useless without Grunt installed, a Sails hook is pretty much useless without Sails core installed.

(B) Do it like Lodash

A slightly more well-rounded variation on (A) is to do all that, but make it optional. In other words, the sails npm package continues to bundle all standalone core hooks as dependencies, and continues to house the command-line tool. But you could also install the npm package called sails-core (which is ​_just_​ Sails core) directly in your project. It would include any core hooks we didn’t pull out as separate packages, and it would be a dependency of sails. Similarly, we'd modify sails-generate-new to support custom builds for new Sails apps-- potentially something like:

sails new my-cool-app --without=sockets,grunt

As a side note, entangling core hooks with sails-generate-new in this way provides a nice clean way to prevent the automatic injection of dependencies related to the hook in the new app (e.g. if you generate an app without the grunt hook, then you don't need all the grunt-contrib-* dependencies; and if you're not using the orm hook, you don't need sails-disk; etc.).

(C) Preinstall

Alternatively, we could get into some weird, hair-raising experimental stuff. Let me preface this by saying I don't know whether this even would work or not, it's just an idea. But here goes:

We could use an NPM preinstall script to install all core hooks as dependencies of Sails core itself, but only in certain situations; i.e. probably only if Sails is being installed globally AND the sails_dont_install_core_hooks environment variable is not set to something truthy. Otherwise, the preinstall script would look for special notation under the sails namespace in the package.json file to determine which hooks should be set up as dependencies of this local Sails app, then modify the Sails package.json file so that the appropriate hook dependencies are installed.

In other words, everything would work as-is, and your project would still depend on a version of Sails directly in its package.json file, with no direct dependencies on hooks. And when you run npm install in your project (e.g. on a new laptop or server), all core hooks would still be installed. But if you had included special notation in your package.json file, only the core hooks you specified would be installed and enabled.

2. Which hooks?

Regardless of which installation strategy from above is being used, we could pull out a only a small subset of certain major hooks: e.g. orm, blueprints, grunt, and sockets (and eventually, http and views). Also there are certain hooks (like logger) that could easily just be swallowed to become part of Sails core directly (since they're used inside of Sails core outside of hook code anyways).

3. Repos

There is a maintenance burden to having separate repos for different aspects of Sails core. The strategy from point #2 would make this easier, and probably worth it, but it's still something to consider. We also don't necessarily have to use separate repos for different hooks to make them separate NPM packages-- but making that work would take some research into how other projects of our scale have done it cleanly. If we only pull out the heaviest hooks as mentioned above, this becomes less of a big deal.

I look forward to reading your feedback. In particular, if you've run into a use case where you needed this in the past, it'd be helpful to hear more about that. Thanks!

[yannbertrand]

I would prefer the second solution which I find the neatest

[mikermcneil]

@yannbertrand ok awesome, same here. Thanks!

[RWOverdijk]

I like the --no-frontend option, which is closest to option B, so I'd go with B.

[Josebaseba]

I like the B option too, but what about the option D, showing in the terminal the hooks that you can enable/disable, and you only need to select or unselect them.

More or less I think that it would work like the B option but a little bit more visual, for the new people that arrives to Sails could be a better way to understand what's going on. Don't you think?

[japel]

yeah! I also like B the most...

[sgress454]

@Josebaseba A more interactive sails new would indeed be an interesting project for someone!

[Salakar]

@mikermcneil Could the .sailsrc be leveraged the way babel 6 works for plugins/presets in it's .babelrc file.

Sails generate could then for example write a custom .sailsrc based on the user provided options and just add in any hook npm dependencies to the generated package.json?

The core hook disabling is already done via that so just thought extending that to handle core hook federation made sense.

Something like:

.sailsrc

{
  "hooks": [
    "csrf",
    "grunt",
    "blueprints",
    "pubsub",
    "sockets"  // automatically added for `pubsub`
  ]
}

and,

optionally read from package.json

{
  "name": "my-sails-app",
  "version": "1.0.0",
  "sails": {
    // my sails rc config here
  }
}

So hooks being an array instead of the old object true/false setup. Only hooks specifically mentioned here get loaded and potentially apply it to userland hooks as well?

The generator could even take this a step further for core hook sub dependencies, you've asked for pubsub so it additionally adds in sockets for you because it's requirement of pubsub.

Just some ideas 💭 , but core hook federation would definitely be good, I create a lot of worker servers that only need hooks, orm and services, the rest sitting there feels bloaty to me. Also at the moment I'm forced to have http and sockets always as I cannot disabled sockets:

.sailsrc file completely ignores the disabling of it (sockets and sails-hook-sockets set to false, nothing works)

Http and sockets hooks please stahhhp.

But that's another issue 😄

[AlexisNo]

I like B too!
@Josebaseba @sgress454 It would be very nice to have a more interactive sails new indeed! But I think we also should keep the possibility to pass the options to the command line too to permit scripted execution.

[mikermcneil]

Http and sockets hooks please stahhhp.
But that's another issue
@Salakar It definitely should be created as one! 🙏

I create a lot of worker servers that only need hooks, orm and services, the rest sitting there feels bloaty to me
@Salakar thanks for providing info about your use case, helps a lot. Our team has had the same experience.

The core hook disabling is already done via that so just thought extending that to handle core hook federation made sense.

@Salakar This is supported today pretty much as you're suggesting using the loadHooks config-- but it doesn't automatically modify the sailsrc file to add dependencies for you if you try to lift with a missing dep (think that might be bad). Love the idea of hooks which are dependencies being auto installed though (i.e. during sails new). I think it would make sense to do the same thing for removing hooks as well (e.g. if you create a new sails app without the sockets hook, then blueprints and pubsub should also be disabled). But in general re: the additive approach, while we could make it so that you could specify either with or without, I think exposing with might be confusing for folks because many hooks have optional dependencies. For instance, the sockets hook can work without sessions enabled. So unless a user really knows what they're doing, I'd rather they specifically call out what they don't want (otherwise I can see people getting excited about custom builds, installing with='blueprints', and then proceeding to have sessions not work).

I like the --no-frontend option
@RWOverdijk Cool, I think we should continue to support that option regardless (It has yet to be officially documented though).

option D, showing in the terminal the hooks that you can enable/disable, and you only need to select or unselect them.
@Josebaseba I'd be down for the CLI to get all interactive on this stuff (I think we should write that up as a separate proposal though, that's too much to spec out here). More on that below.

But I think we also should keep the possibility to pass the options to the command line too to permit scripted execution.
@AlexisNo if we ever go with an interactive prompt, we'll definitely need to be sure you can quell the prompts using either cli opts, cli args, or env vars. Also, it's likely that for consistency, we'll eventually change the CLI to work like https://github.com/treelinehq/treeline/blob/master/bin/treeline-export.js using machine-as-script. It standardizes how CLI args and opts are passed in, and makes it easy to use env vars.

[Salakar]

@mikermcneil You're right, it should also do the removing of the hooks also.

In general the generator / cli I think should provide a clear picture of what the core hooks are, what optional features that specific core hook can have (soft dependencies) and what hard dependencies are required. Initially, from my experience this has been a guessing game, disable X hook wait for sails to load and maybe throw an error about missing a core hook. (perhaps just needed more documentation)

Something else that could be good is preset core hook configurations, i.e. api, web, worker and so on for the common use cases, and then proceeding to optionally customise chosen preset. Internally we have a repo for each of those configurations that we clone as a base project. But lately we tend to use this sails generator for api project configurations, the generator makes life so much easier.

Another random thought, sorry 😸 - for on the sails site, a customise build wizard on the getting started guide that generates the sails new command for you to run, copy, paste and done. I imagine all the options of disabling/enabling/this requires that etc could get confusing to some. I'm happy to mock something like this up in react, just need a clear list of what requires what, and what hooks can optionally use other hooks for additional functionality.

Praise the almighty text wall. \o/

[mikermcneil]

@Salakar agreed- we need a declarative encoding of hard and soft hook interdependence. The first step towards that is formally documenting it in the various hooks (e.g. https://github.com/balderdashy/sails/tree/master/lib/hooks/responses#dependencies). I've tried to use that format as a standard template for the core hooks I've made it through so far-- still some left go do.

Another random thought, sorry - for on the sails site, a customise build wizard on the getting started guide that generates the sails new command for you to run, copy, paste and done. I imagine all the options of disabling/enabling/this requires that etc could get confusing to some. I'm happy to mock something like this up in react, just need a clear list of what requires what, and what hooks can optionally use other hooks for additional functionality.

That sounds awesome! I'd rather not add a react dependency to the Sails site right now (we're already loading Angular on the page), but this sounds like it'd be a really helpful tool.

Something else that could be good is preset core hook configurations, i.e. api, web, worker and so on for the common use cases, and then proceeding to optionally customise chosen preset. Internally we have a repo for each of those configurations that we clone as a base project. But lately we tend to use this sails generator for api project configurations, the generator makes life so much easier.

That's helpful, thanks. I think that would be a good v1 enhancement to spec out. I'd be 100% down to make something like sails generate worker as part of core (would just need to figure out exactly what that means)

[mikermcneil]

By the way, @everyone:

Thanks! There's been some great feedback and ideas so far. If you haven't chimed in yet, please feel free to share your use case and any related thoughts on usage. This will be a key part of the Sails v1 release later this year, so it's important we get it right.

The next step is to start detailing everything that would need to happen in tests, docs, generators, and Sails core for this to get implemented. I won't have time to look over this until after we release v0.12, but in the mean time you can help move this along by putting together a list of where the various changes would need to take place in sails-generate-new, sails, and sails-docs and posting it here. Once we've got all the usage details like exit conditions and so forth specified here, I'll merge this proposal in to the backlog. Then we can start writing code and get this into core asap!

[mikermcneil]

As an addition, here's a few relevant snippets of the conversation @sgress454 and I just had about this:

On a related note re teardown, there’s an open question as to whether `moduleloader` should stay its own hook at all, or be broken apart into the places where it is called by `controllers`, `orm`, etc.  I think there’s a really good argument to make it ​_not_​ be that way , it’s just a matter of whether it’s worth it.

sgress454 [12:01 AM] 
it’s already essentially separate—it (and userconfig, and userhooks) all load individually when Sails starts. and it doesn’t use much of the hook api

mikermcneil [12:01 AM] 
the original point was to be able to override all module loading at once, which is still valid, but it creates a bunch of unnecessary interdependence. Also it ends up being a dependency of like every hook.  And realistically, every hook would still want to wait for `userconfig` if it is enabled, but if it is not present, they can just ignore it (optional dependency)

sgress454 [12:02 AM] 
https://github.com/balderdashy/sails/blob/master/lib/app/private/loadHooks.js#L113-L144

The reason I bring this in here is in the context of a more general discussion about hook dependencies. There really aren't that many hard, required hook dependencies (most dependencies are optional). But if we were to indicate them declaratively as part of the hook spec (I'm talking just hard dependencies, not optional ones), it would make for a much better experience when disabling/enabling core hooks in your apps and for higher-level tooling.

See #1039 for more background

[atiertant]

@mikermcneil i would prefer an extra package by hook... does core hook could be replaced by a custom one and interact with other's core:
sails-hook-orm-offshore would replace completely the core orm hook and then could be used by blueprints without having to rewrite a custom blueprint hook

[mikermcneil]

@atiertant totally agree! If I'm understanding you correctly, what you're suggesting will work in the currently released version of Sails-- just be sure to take a look at sails-hook-orm's package.json and include the sails.hookName key in your package.json file (and set it to orm-- see https://github.com/balderdashy/sails-hook-orm/blob/master/package.json#L35). Provided it exposes the same API as the core orm hook, what you're saying should work! See sails-hook-orm-mongoose for another example (bear in mind that particular hook does not implement all of the same methods though)

[mikermcneil]

@atiertant @AlexisNo @yannbertrand @Josebaseba @RWOverdijk @japel @Salakar Hey y'all, this is now implemented on the edge prerelease of Sails v1 (npm install sails@edge -g). If you install the edge release, you'll be able to generate a new app with sails new foo --without=grunt,orm,sockets,lodash,async, or any combination. Would you try it out and let me know what you think?

[luislobo]

HI Mike, I just tested it like this: sails new foo --without=grunt,orm,sockets
Some comments about this one: The home page is not generated. no /view/homepage.ejs exists, so, a message is shown: error: Ignoring attempt to bind route (`/`) to unknown view: `homepage`
Also, when you go to sails console, an empty entry is shown in the list of hooks:

moduleloader,logger,request,views,blueprints,responses,helpers,,policies,services,security,i18n,userconfig,session,http,userhooks

Notice the "empty" space between helpers and policies. I wonder if this might be an issue.
And the last thing, the --without parameter should be documented in the sails new --help
Today it just shows:

  Usage: new [path_to_new_app]

  Options:

    -h, --help                 output usage information
    --viewEngine [viewEngine]  
    --template [viewEngine]    

[luislobo]

Now that I'm thinking it more, does removing grunt makes the views not publishable? I noticed also that the assets folder is not there, neither. What would become the "public" folder when running without grunt?

BTW, adding a views/homepage.ejs, fixes the routing issue, and it just works ok.

[sgress454]

@luislobo Thanks for your feedback!

The home page is not generated. no /view/homepage.ejs exists, so, a message is shown: error: Ignoring attempt to bind route (/) to unknown view: homepage

We're currently not generating the homepage when ORM and Sockets hooks are disabled, for the simple reason that the current homepage has text on it that talks about generating an API and accessing it via WebSockets. There's a TODO in the generator to come up with a stripped-down version of the homepage; I'll make sure it gets added to our list (or at the very least, that the default / route is not generated in these circumstances).

Also, when you go to sails console, an empty entry is shown in the list of hooks:

The empty spot is probably for the "PubSub" hook, which is false in this scenario since it turns itself off if ORM or sockets is not available. I'm curious as to how you're generating that list?

the --without parameter should be documented in the sails new --help

Good call, thanks for the heads up!

[luislobo]

I just got the list typing sails ;)  From: notifications@github.comSent: December 5, 2016 10:11 AMTo: sails@noreply.github.comReply-to: reply@reply.github.comCc: luislobo@gmail.com; mention@noreply.github.comSubject: Re: [balderdashy/sails] [Proposal] Federate core hooks (#3504) @luislobo Thanks for your feedback! The home page is not generated. no /view/homepage.ejs exists, so, a message is shown: error: Ignoring attempt to bind route (/) to unknown view: homepage We're currently not generating the homepage when ORM and Sockets hooks are disabled, for the simple reason that the current homepage has text on it that talks about generating an API and accessing it via WebSockets. There's a TODO in the generator to come up with a stripped-down version of the homepage; I'll make sure it gets added to our list (or at the very least, that the default / route is not generated in these circumstances). Also, when you go to sails console, an empty entry is shown in the list of hooks: The empty spot is probably for pubsub, which is false in this scenario since it turns itself off if ORM or sockets is not available. I'm curious as to how you're generating that list? the --without parameter should be documented in the sails new --help Good call, thanks for the heads up! —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.