8000 __hooks become writable and configurable by bertho-zero · Pull Request #1520 · feathersjs/feathers · GitHub
[go: up one dir, main page]

Skip to content

__hooks become writable and configurable#1520

Merged
daffl merged 2 commits intomasterfrom
bertho-zero-patch-1
Oct 10, 2019
Merged

__hooks become writable and configurable#1520
daffl merged 2 commits intomasterfrom
bertho-zero-patch-1

Conversation

@bertho-zero
Copy link
Contributor

We should be able to modify or delete this value.

In a particular case it will avoid me this kind of error:

Uncaught TypeError: Cannot redefine property: __hooks
Uncaught TypeError: Cannot delete property '__hooks' of #<Object>

We should be able to modify or delete this value.

In a particular case it will avoid me this kind of error:
```
Uncaught TypeError: Cannot redefine property: __hooks
```
```
Uncaught TypeError: Cannot delete property '__hooks' of #<Object>
```
@daffl
Copy link
Member
daffl commented Aug 22, 2019

What would be the use case for this? It's usually not ideal having to modify undocumented framework internals in an application. For skipping hooks we can now e.g. use the adapter hook-less service methods.

@bertho-zero
Copy link
Contributor Author
bertho-zero commented Aug 23, 2019

I wish I could edit or delete hooks, in my case I use uberproto to mixin a service in a const. This is not a normal use of Feathers but why protect these values?

const Users = require('../services/users');

const service = Object.create(Users.prototype);

module.exports = service; // like a singleton ¯\_(ツ)_/¯

module.exports.init = () => {
  const service = feathers()
    .use( '/', new Users({ /* options */ }) )
    .setup()
    .service('/')
    .hooks(hooks);

  Proto.mixin(svc, service);
};

This is not a choice but an imposed technical constraint.

However, I do not have these errors anymore.

@bertho-zero bertho-zero deleted the bertho-zero-patch-1 branch September 10, 2019 13:49
@bertho-zero bertho-zero restored the bertho-zero-patch-1 branch September 23, 2019 18:36
@bertho-zero bertho-zero reopened this Sep 23, 2019
@bertho-zero
Copy link
Contributor Author

This property just needs not to be enumerable.

@daffl daffl merged commit 1c6c374 into master Oct 10, 2019
@daffl daffl deleted the bertho-zero-patch-1 branch October 10, 2019 01:40
@daffl
Copy link
Member
daffl commented Oct 10, 2019

Similar to my comment in #1521 but I guess this one makes sense for testing purposes. Although this will change in the future anyway via #932.

@bertho-zero
Copy link
Contributor Author

It's actually in a test suite that it stuck, because of the init called several times.

These are internal values but if we want to modify or delete them in cases of advanced or atypical use we should be able to do it. #1520 (comment)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0