Doesn't work with destructured arguments

[TooTallNate]

I'm not exactly sure what the format for destructured Arrays and Objects should be, but they're definitely not supported currently.

For example:

function foo([a, b], { c, d }) {
}

console.log(functionArguments(foo));

// actual
[ '[a', 'b]', '' ]

// possibly expected?
[ [ 'a', 'b', ], { c: true, d: true } ]

Thoughts?

[ORESoftware]

@TooTallNate maybe use this instead? https://github.com/tunnckoCore/parse-function

I believe @charlike wrote the above, so all in the family...are you creating a DI library by the way? Curious what your use case is.

[tunnckoCore]

TooTallNate, yes it is not supported here, and won't be. This package is smallest and simple approach.

in parse-function is also not supported currently but it is exstensible with plugins and different parsers. there is open issue for what should be the result, because we are not sure too

[TooTallNate]

Couldn't it remain simple, but still be updated to support ES6 syntax?

[TooTallNate]

Related: https://github.com/tunnckoCore/parse-function/issues/47

[TooTallNate]

@ORESoftware

are you creating a DI library by the way?

Yes.

[tunnckoCore]

dont know, it is regex based so..

prs welcome anyway if it's someting small and dont add much code/deps.

[ORESoftware]

@TooTallNate uhh, well if you are creating a DI library, destructured args won't really work will it :)

think about it

function({a,b}){}

how iz yer DI lib gonna know what the name of the encompassing object is?

this will work of course:

function(c){
    const {a,b} = c;
}

[TooTallNate]

The DI lib only needs to know that the user is using a and/or b and then supply the arguments. I wouldn't expect them to use c as a parameter name for this particular case (it would probably throw an Error).

[tunnckoCore]

Hm, pretty easy implementation using balanced-match.

const balancedMatch = require('balanced-match')

function functionArguments (fn) {
  if (typeof fn !== 'function') {
    throw new TypeError('function-arguments expect a function')
  }
  if (!fn.length) {
    return []
  }
  let fnToStr = Function.prototype.toString
  let fnStr = fnToStr.call(fn)

  // from https://github.com/jrburke/requirejs
  let reComments = /(\/\*([\s\S]*?)\*\/|([^:]|^)\/\/(.*)$)/gm
  fnStr = fnStr.replace(reComments, '') || fnStr

  let res = balancedMatch('{', '}', fnStr)

  // when destructing arguments
  if (!res.post.length) {
    const pre = res.pre.replace(reComments, '') || res.pre
    res = balancedMatch('(', ')', pre)
  }

  if (!res.body.length) {
    return []
  }

  return res.body.split(',').map((str) => str.trim())
}

examples

const foo1 = (a, b, c) => {
  if (a) {
    return a + 1
  }
}

const foo2 = ({ a, b, c }) => {
  if (a) {
    return a + 1
  }
}

const foo3 = function (a, b, c) {
  if (a) {
    return a + 1
  }
}

const foo4 = function ({ a, b, c }) {
  if (a) {
    return a + 1
  }
}

console.log(functionArguments(foo1)) // => [ 'a', 'b', 'c' ]
console.log(functionArguments(foo2)) // => [ 'a', 'b', 'c' ]
console.log(functionArguments(foo3)) // => [ 'a', 'b', 'c' ]
console.log(functionArguments(foo4)) // => [ 'a', 'b', 'c' ]

Are these results okey? Using balanced-match (which i was thinking to use from the start) is easy and simplifies the things, but not sure for the performance and such.

Should we add an option to control this? For example destructuring as second argument, so it will return an array of destructed arguments only when is true?

[tunnckoCore]

I think destructuring is pretty fantastic and perfect for enforcing conventions and i'm using it always, but not so good for DI, don't know.

[ORESoftware]

@TooTallNate sorry I didn't really follow what you meant...ok now I know what you mean. So it will see a and b, and then wrap those two in an object. But then I don't see what the point of destructuring args is then.

instead of this:

function({a,b}){}

just expect your users to use:

function(a,b){}

[ORESoftware]

amirite or amirite?

I ask because I am creating a DI lib too and interested in making it as usable as possible

I'd venture to guess 90% of people interested in function-arguments will be wanting to make DI possible

[TooTallNate]

@ORESoftware Ya it's a good point. One of the benefits with the desctructured object approach is that you can destructure further in the function signature. So if I'm using user I can destructure it like:

function ({ user: { id, name } }) {
}

Whereas with named parameters I'd have to do it in another line:

function (user) {
  const { id, name } = user;
}

[ORESoftware]

Interesting, yeah if you find a way to get destructuring to work well with DI, I'd like to know, thanks

[ORESoftware]

Like I was saying above I don't know how a DI lib of any kind would know that user is supposed to be injected when you do:

function ({ id, name }) {
}

it's not really possible to know that id and name are supposed to be extracted from the user object.

The way my DI lib works, is there is a list of injectables:

const injectables = {
  foo: Promise.resove({}),
  bar: {},
  baz: function(cb){ 
        setTimeout(function(){
         cb(null, {});                     
        }, 100)
      }
}

If my lib can't find an injectable by key, it will try to require() the value. The user configures the injectables themselves, and the main win is it can handle async deps, as indicated.

So now I know what to inject when I have:

function (foo, bar, baz) {
}

[tunnckoCore]

That's why real parser is needed, and parse-function is perfect for this job, because has plugins. So anyone can extend further. And that's why i almost always close issues here, this one is meant to be so simple and small.

[TooTallNate]

If you're going to close the issue I'd at least mention in the README that ES6 features will not be supported, by design. I agree that the AST approach will be easier to implement.

[tunnckoCore]

I'd at least mention in the README that ES6 features will not be supported

Ah, yea, good point :100:

Okey, thanks for opening this issue, but please try to create plugin for parse-function and then we can list it in the readme :)

If you need some help or something, feel free to ask. I should add some notes to its readme soon, until then you can see the readme of v4.

[ORESoftware]

There's always this sort of pattern

 const {Test} = suman.init(module, {
   $inject: ['a', 'b', 'c']
 });

 Test.create(function($inject){
    const {a,b,c} = $inject;
 });