feathers-plus / cli

FeathersJS CLI supporting both REST and GraphQL architectural concepts and their query languages.
https://generator.feathers-plus.com/
Other
43 stars 7 forks source link

regenerating service removes all hooks on service #29

Closed lunchboxer closed 5 years ago

lunchboxer commented 5 years ago

Steps to reproduce

Generate a service add a hook to servicename.hooks.js run feathers generate service

Expected behavior

feathers generate service should leave servicename.hooks.js alone if it already exists. It doesn't add anything or update it, does it?

Actual behavior

hooks are erased from servicename.hooks.js. This is pretty annoying. If I forget about this behaviour then I don't get an alarm going off, but my app silently behaves in a way different from how I set it up.

System configuration

Ubuntu 18.10 Node 10.13.0

@feathers-plus/cli 0.7.76

eddyystop commented 5 years ago

servicename.hooks.js is regenerated to import the newly created hook.

You can only add custom code at the locations code code is allowed.

lunchboxer commented 5 years ago

Where is that? I would expect to register my hooks as below, but these get overwritten, the "DEFAULT" tag is a good hint of that already. but Where am I supposed to register my hooks then. This seems like especially strange behaviour since all the generator does is clear it and add authenticate. I have apparently misunderstood the intended workflow with hooks, but it isn't lack of trying.

let moduleExports = {
  before: {
    // Your hooks should include:
    //   all   : authenticate('jwt')
    // !<DEFAULT> code: before
    all: [authenticate('jwt')],
    find: [],
    get: [],
    create: [addTimestamp()],
    update: [],
    patch: [],
    remove: [],
    // !end
  },

  after: {
    // !<DEFAULT> code: after
    all: [],
    find: [],
    get: [],
    create: [limitEntries()],
    update: [],
    patch: [],
    remove: [],
    // !end
  },

In the docs there is mention positioning generated hooks but I think I'm missing one or two more necessary clues.

Am I supposed to duplicte the moduleExports object in the block labeled ' // !code: moduleExports // !end' and then put my hooks in there?

eddyystop commented 5 years ago

We expect users to customize the code insertion points when they are adding hooks. So you would remove the two <DEFAULT> in your example.

The downside is that potentially later a generate may want to add default hooks to what was originally there, but the generator can't do so because the code insertion point is no longer the default. That is why the default hooks are always also listed as comments e.g. // all: authenticate('jwt'). Those comments indicate which hooks should be in the code insertion point.

Customizing moduleExports later would be awkward because the order of multiple hooks is important.

lunchboxer commented 5 years ago

Thanks for helping me with this. I would like to suggest that the docs be made slightly more explicit about this step. I found it confusing as to what the intended behaviour of the generator is and how a user might best work with it.

eddyystop commented 5 years ago

Projects improve through contributions. Please consider making a PR for this.