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

[Bug] Overriding the core ORM and Sockets hooks is difficult with npm v3 and later #3855

Closed
jeremy-albuixech opened this issue Oct 18, 2016 · 2 comments
Labels

Comments

@jeremy-albuixech
Copy link

jeremy-albuixech commented Oct 18, 2016

Sails version: 0.12.7
Node version: 6.7.0
NPM version: 3.10.8
Operating system: Mac OSX El Capitan


To override a core hook, you can create a new module and name it similarly as the core hook but under a different scope (for example, @ioredis/sails-hook-sockets) and with the sails.isHook = true in the package.json.

In the moduleLoader hooks library of Sails, the nodeModulesFolder function goes through the node_modules folder in order to search for hooks to load, with a max depth of 3.

The issue is that with npm v3, the node_module folder is flattened, so the max depth does not apply. For example, since sails requires the sails-hook-sockets module, instead of being under node_modules/sails/node_modules/sails-hook-sockets, it's now directly under /node_modules/sails-hook-sockets.

What it means is that if your custom hook has a sort order inferior (alphabetically I guess) in regards to the core hook, the custom hook will get loaded first and then overwritten by the core hook.

To reproduce you can follow these steps:

  1. Include the version of the sails-hooks-socket we use: https://github.com/BroadsoftLabs/sails-hook-sockets to your project's package.json
  2. Make sure you have nothing under the node_modules folder (rm -rf node_modules) and do a npm install
  3. Lift your sail application with the verbose log level to see all the user-related hooks logging information

You will see the following logs:

screen shot 2016-10-18 at 2 36 07 pm

What's happening here: sails finds my custom hook (under node_modules/@bsft/sails-hook-sockets) but then finds the core hook (under node_modules/sails-hook-sockets), so it loads that one instead.

The same issue probably applies to the ORM hook since it's also required as a dependency and not part of the sails core libraries.

I will try to contribute through a PR to fix this issue.

I created a repository here that you can use to quickly reproduce the issue: https://github.com/albi34/sails-issue-hooks

@mikermcneil
Copy link
Member

@Albi34 Thanks for the detailed explanation - will take a look this afternoon.


As a temporary workaround, if, in your app.js, you modify the code such that you pass in sails.config.hooks programmatically:

  // Start server
  sails.lift(require('lodash').extend(config,
    hooks: {
      sockets: require('uchub-sails-hook-sockets')
    }
  }));

then you should be able to override the default by lifting with node app.js.

@sgress454
Copy link
Member

Alright, this is patched on master -- the hook loader will ignore the default installed hooks (sails-hook-sockets, sails-hook-orm and sails-hook-helpers) so that they can be safely overridden. The workaround should get you going on v0.12 until then. Thanks for posting @Albi34!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants