feathersjs / hooks

Async middleware for JavaScript and TypeScript
MIT License
157 stars 12 forks source link

function.length is lost #76

Closed bertho-zero closed 3 years ago

bertho-zero commented 3 years ago

function.length is lost, and I see an error calling hooks without hooks argument.

const { hooks } = require('@feathersjs/hooks');

const emptyHook = async (context, next) => next();

// Hooks can be used with a function like this:
const sayHello = async name => {
  return `Hello ${name}!`;
};

const sayHello2 = hooks(async name => {
  return `Hello ${name}!`;
});

const sayHello3 = hooks(async name => {
  return `Hello ${name}!`;
}, []);

const sayHello4 = hooks(async name => {
  return `Hello ${name}!`;
}, [emptyHook]);

(async () => {
  console.log(sayHello.length); // 1
  console.log(sayHello2.length); // 3, should be 1
  console.log(sayHello3.length); // 0, should be 1
  console.log(sayHello4.length); // 0, should be 1

  console.log(await sayHello('Bertho'));
  // console.log(await sayHello2('Bertho'));
  // Throw an error: TypeError: manager.parent is not a function
  //  at Object.setManager
  console.log(await sayHello3('Bertho'));
  console.log(await sayHello4('Bertho'));
})();
bertho-zero commented 3 years ago

The same may be happening with function.name, I haven't checked.

daffl commented 3 years ago

Hm, so according to MDN the property is not writeable but configurable. I tested this and it looked like it worked (but not sure if that applies to all engines):

function blabla (a, b, c) {}
Object.defineProperty(blabla, 'length', { value: 5 })
console.log(blabla.length);
bertho-zero commented 3 years ago

image

The configurable seems newer than the function.length, and still not supported on IE.

This redefinition of length and name must be in a try catch in my opinion.

bertho-zero commented 3 years ago

image

As expected, it's broken in IE.