Skip to content

feat(hooks): project persistent hooks #5597

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

Merged
merged 3 commits into from
Oct 25, 2021

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Oct 21, 2021

PR Checklist

What is the current behavior?

Because the hooks folder in a NS project is not meant to be checked into source, and is cleared with ns clean, there is no easy way to have project persistent hooks aside from creating a local node_module plugin that registers the hook.

What is the new behavior?

These changes enable specifying project hooks in the nativescript.config.(ts|js) file and the cli will detect the configuration during hook events (before/after). I borrowed the same hook config syntax that plugins specify in their package.json for simplicity.

Just add the following config to enable project persistent hooks for your app.

export default {
  id: 'org.nativescript.app',
  main: 'app.js',
  hooks: [
    {
      type: 'after-prepare', // one of the hook events
      script: 'scripts/some-script.js', // can be any path in the project
    },
  ],
};

Fixes/Implements/Closes #5576.

There is greater discussion in this issue regarding a better way to organize plugin hooks, but I only attempted to just permit project persistent hooks within the current hook flow.

@cla-bot cla-bot bot added the cla: yes label Oct 21, 2021
@NathanWalker
Copy link
Contributor

A very humble thank you 🙏 ☺️

@NathanWalker NathanWalker requested a review from rigor789 October 21, 2021 04:03
Copy link
Member

@rigor789 rigor789 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome PR, thanks!

One small suggestion is to use the second parameter for defaultValue when using getValue (typings may be off - missing the second param, I pushed a fix to them yesterday).

The other one is to scope the hooks array under the cli object. This one I'm not 100% sure about, but in #5596 we added the packageManager option under the cli scope since we plan to extend this to more options in the near future - and it's nice to group CLI overrides/function under a single key. However, hooks are arguably a CLI thing, but also tightly coupled to a project, so it may be fine to leave it at the root level.

Thoughts on this are welcome! (now that I typed this, I'm leaning towards a root hooks array like you've done - reverted my suggestion)

@rigor789 rigor789 merged commit 97bbe33 into NativeScript:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Place for project-persistent hooks
3 participants