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

[Patch] Update regex to parse hook names from module identities #3514

Merged

Conversation

lennym
Copy link
Contributor

@lennym lennym commented Jan 21, 2016

This allows the hook names to be correctly parsed from modules published to npm under a user's namespace/scope - e.g. @user/sails-hook-name

Follows discussion in #3502

This allows the hook names to be correctly parsed from modules published to npm under a user's namespace/scope - e.g. `@user/sails-hook-name`
@Salakar
Copy link

Salakar commented Jan 25, 2016

I updated my comment on the issue, the regex can be reduced further to the below:

/^(?:@.+\/)?(sails-hook-)/

See: https://regex101.com/r/lC8sM4/2

Sorry! :) But then again either way works 👍

@mikermcneil
Copy link
Member

@lennym nice- this was the intended behavior (unless an identity is specified in the hook def, in which case that's used). Do you think you could tweak this pr to support using that if it's not undefined? Happy to help if I can-- this is what we need to be able to properly override core hooks via npm installed hooks (eg If you npm install an experimental version of sails-hook-orm-mongoose in your app that is fully compatible with blueprints and pubsub, then that hook needs to expose its identity as 'orm' ). @sgress454 and I can help with tests and docs on this if you don't have time (we were wanting to get this in anyway)

@mikermcneil
Copy link
Member

And @Salakar, thanks for helping review!

@mikermcneil mikermcneil changed the title Update regex to parse hook names from module identities [Patch] Update regex to parse hook names from module identities Jan 25, 2016
@mikermcneil mikermcneil added the patch This label is deprecated. Please don't use it anymore label Jan 25, 2016
@sgress454
Copy link
Member

Working on this now, adding some tests and should have it merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch This label is deprecated. Please don't use it anymore
Development

Successfully merging this pull request may close these issues.

4 participants