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

__hooks become writable and configurable #1520

Merged
merged 2 commits into from
Oct 10, 2019
Merged

__hooks become writable and configurable #1520

merged 2 commits into from
Oct 10, 2019

Conversation

bertho-zero
Copy link
Collaborator

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
Collaborator 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
Collaborator 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
Collaborator 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