Skip to content

Place for project-persistent hooks #5576

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

Closed
rigor789 opened this issue Sep 22, 2021 · 7 comments · Fixed by #5597
Closed

Place for project-persistent hooks #5576

rigor789 opened this issue Sep 22, 2021 · 7 comments · Fixed by #5597
Labels
hooks Describes issues related to hooks ux

Comments

@rigor789
Copy link
Member

Trying to do something similar I ended using hooks. I feel the use of hooks should be easier and could achieve this issue as well. Since we all are developers, a programmatic hook to make any changes on the project is flexible and powerful (not so clean, though).

But I feel these issues:

  • Very few docs, and only on the CLI repo, should be linked or mentioned in webpage and/or here. When you search for nativescript hooks in google or in the issues list, you won't find anything relevant! Only a blog post talking about v6 migration.
  • Document or simplify the API to know how to retrieve basic information (the inners of CLI can be hidden a bit only for those willing to risk versions breaking their hooks).
  • hooks directory is usually ignored for repo and removed (by tools or by user), a place to put project persistent hooks would be great.

Should this be a separate issue?

Originally posted by @xiromoreira in NativeScript/NativeScript#9520 (comment)

@jcassidyav
Copy link
Contributor

This would be great, the way I do this today is to have a folder containing the hook in the project, which has a package structure writted in js and then reference it in the package.json of the project with file:// works well for the very simple hooks I need.

@rigor789
Copy link
Member Author

rigor789 commented Sep 22, 2021

My take on this is that plugin hooks should not be handled inside the hooks folder. The original thought I believe was to allow plugins to add hooks, but let the user essentially control them: allow deleting them, disabling them etc. However, since hooks are generally installed by a postinstall script, there never was an easy way to opt-out of a hook from any given plugin/dependency.

We started ignoring the hooks folder, as well as auto-cleaning it with ns clean just because hooks are generally considered part of a 3rd party dependency, and if that dependency changes, there's a chance a hook may get removed from the dependency and then cause issues when the CLI is not able to find the hooks referenced by the scripts inside the hooks folder.

In some cases though as you said, it is useful to write custom hooks for a given project, and right now there's no easy/clean way to do so without making a plugin (could be a local folder referenced in the package.json, but still hacky).

Ideally, the hooks folder should be checked-in the repo, and not cleaned by ns clean, and for that to happen their inner working should shift to staying in node_modules and no postinstall scripts. The remaining question is, how to allow apps to opt-out of a hook from any given plugin?

@rigor789 rigor789 added hooks Describes issues related to hooks ux labels Sep 22, 2021
@jcassidyav
Copy link
Contributor

jcassidyav commented Sep 22, 2021

Given what is there today, the code which actually executes the plugins could be configured to not run particular ones e.g.
for todays implementation (hooks dir), config could be added to the nativescript.config.ts

 hooks: {
    exclude: [
      {
        event: "after-prepare",
        hooks: [ "nativescript-unit-test-runner", "nativescript-firebase" ]
      }
    ]
  }

or if in a new format executed from node_modules

  hooks: {
    exclude: [
      {
        
        plugin: [ "@nativescript/unit-test-runner"],
        events: ["after-prepare"]
      },
      {
        
        plugin: [ "@nativescript/firebase"],
        events: ["after-prepare"]
      },
    ]
  }

@rigor789
Copy link
Member Author

My main concern is that technically hooks can "come and go" from plugins, so long-running projects could end up with a bunch of unused/stale configuration.

Just speculatively, but what if the CLI would keep track of the plugins, and their hooks automatically - ie. when a plugin adds a hook, it's added to the config and enabled by default, but once you change it, it stays disabled (won't override). If a plugin removes a hook, the CLI would remove it from the config as well, since it no longer applies anyways.

plugins: {
  '@nativescript/unit-test-runner': {
    hooks: {
      'after-prepare': false // disabled hook
    }
  }
  '@nativescript/firebase': {
    hooks: {
      'after-prepare': true // true by default, user can set to false to disable
    }
  }
}

This feels a bit verbose, and I don't like the true/false value there, but that's a minor detail we can play around with.

@jcassidyav
Copy link
Contributor

jcassidyav commented Sep 22, 2021

To me that feels like a return to the webpack.config.ts problem where something is modifying a file under my source control just because I installed it or a new version of it, personally I'd prefer these files to be as minimalist as possible with only explicit deviance from the default that I put in.

I seems to me that wanting to ignore a hook added by a plugin would be an edge case? i.e. the plugin must need the hook for something in order to work properly.

In addition, you could have configuration that is like this to enable project hooks. i.e. a separate discovery mechanism for project hooks.

 hooks: {
    include: [
      {
        event: "after-prepare",
        hooks: [ "./mypathtomyhook/some-hook-i-wrote"]
      }
    ]
  }

@rigor789
Copy link
Member Author

rigor789 commented Sep 22, 2021

Right, it is an edge case, and I agree - the CLI should not modify code under source control (the config).

What if we simplified it further to just

disabledHooks: [
  '@nativescript/firebase:after-prepare`
]

or

disabledHooks: {
  `@nativescript/firebase`: ['after-prepare'] // or 'after-prepare' without array, or 'all' or even `*`
}

I would also explore simplifying the hooks folder to perhaps a flatter structure, though how that would work is not clear yet.

For custom discovery like you mention, I would go with a similar, simplified syntax

hooks: {
  './path/to/some/hook': [ 'after-prepare' ] // or 'after-prepare' without array
}

The ergonomics would need some more thought, like would it be better to have this schema?

{
  hooks: {
    disabled: { 
      `@nativescript/firebase`: ['after-prepare'] // or 'after-prepare' without array, or 'all' or even `*`
    },
    include: {
       './path/to/some/hook': [ 'after-prepare' ] // or 'after-prepare' without array
    }
  }
}

@jcassidyav
Copy link
Contributor

I guess the config would reflect the scope of the change.

If there is going to be a move away from the hooks directory then that looks good to me,

I presume that would involve changing @nativescript/hooks to record somewhere the hooks the plugin wants to install and where they are, and then changing the cli to look for the actual hooks in node_modules somewhere.

But the cli would have to support the old hooks directory mechanism, to remain compatible with older versions ( plus some plugins seem to write hooks outside the @nativescript/plugin mechanism e.g. the firebase plugin ).

A simpler change would be to keep the existing hooks functionality and just change the cli to ignore/execute based on the config.

Performing the filtering at runtime in that case, you do not know which plugin installed the hook, all you have is the name of the hook file ( which can be anything if a plugin has written it there)

so that scenario is covered by the likes of this

 hooks: {
    exclude: [
      {
        event: "after-prepare",
        hooks: [ "nativescript-unit-test-runner", "firebase-build-gradle" ]
      }
    ],
   include: [
      {
        event: "after-prepare",
        hooks: [ "./mypathtomyhook/some-hook-i-wrote"]
      }
    ]
  }

I like the more verbose configuration because it is self describing, which is of course purely subjective.

And you could argue that manually putting files in hooks folder is unsupported rendering some of the above moot.

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

Successfully merging a pull request may close this issue.

2 participants