function with empty parameters for @plumier/reflect

[sebaplaza]

Hello,

reflect is not working for function empty parameters

Test example

import reflect from '@plumier/reflect';

it('should return empty for a function with none parameters', () => {
    const fn = () => {
      return true;
    };

    // throws an exception here
    const metadata = reflect(fn);
    expect(metadata).toBeDefined();
  });

    TypeError: Cannot convert undefined or null to object
        at Function.getOwnPropertyNames (<anonymous>)

Thanks

[ktutnik]

Hello,

Are you using pure JavaScript? because the typing will prevent you passing function into reflect. Officially reflect only possible to reflect Class and module (string).

Internally the library is possible to reflect a function, but not been tested to reflect a lambda function. Because I was thinking this feature was not needed (from Plumier perspective) but I can add this feature if necessary.

[sebaplaza]

Yes, i'm using it in both contexts (Javascript and Typescript).

I use reflect in a dependency injection system, also to validate the presence of named parameters in multiple situations (with named/lambda/async functions).

It would be great if you can implement this 🙏🏼

Thank you

[ktutnik]

Sure I can add that, but with some limitations, the data type of the parameters and the return type of the function will not be reflected. This is due to the inability of TypeScript to decorate a function. So you will only get the parameter names. Ok with that?

[sebaplaza]

Yes, i'm ok with that. I'll be only using the parameters name.

[ktutnik]

I merge the modification into the master #1029 and built a canary version for you to try.

"@plumier/reflect": "1.0.7-canary.4"

Here are some scenarios that have already been covered.

plumier/tests/behavior/reflect/function.spec.ts

Lines 32 to 55 in f65ec5b

	it("Should reflect function", () => {
	const myFunction = () => { }
	const meta = reflect(myFunction)
	expect(meta).toMatchSnapshot()
	})
	it("Should able to reflect function parameter names", () => {
	const myFunction = (par1: number, par2: number) => { }
	const meta = reflect(myFunction)
	expect(meta).toMatchSnapshot()
	})
	it("Should able to reflect destructured parameter", () => {
	interface Parameter { prop1: number, prop2: number }
	const myFunction = ({ prop1, prop2 }: Parameter) => { }
	const meta = reflect(myFunction)
	expect(meta).toMatchSnapshot()
	})
	it("Should able to reflect rest parameter", () => {
	const myFunction = (par1: number, ...par2: number[]) => { }
	const meta = reflect(myFunction)
	expect(meta).toMatchSnapshot()
	})

[sebaplaza]

Hello, thanks for the updates !

I was testing the new version with some of my tests, and i have this one that is not passing:

  it('should return names parameter of a function', () => {
    const fn = (req, callback) => {
      callback(null, req);
    };

    const metadata = reflect(fn);
    expect(metadata.parameters).toHaveLength(2)
  });

The metadata object is not detecting req and callback parameters.

// metadata
{ 
  "kind": "Function", 
  "name": "fn", 
  "parameters": [] 
}

[ktutnik]

Sorry, my mistake. The tests actually didn't pass, I didn't check the snapshot properly.

Can you try now using the 1.0.7-canary.5 I just publish it.

[ktutnik]

@sebaplaza do you still getting this issue? confirm if it works for you and I will close this issue.

[sebaplaza]

I'm really sorry, i had no time to test yesterday.

Well, is working for the above test ! Thanks

Anyway, i'm facing another failing case, this time with functions defined inside objects

  it('should returns parameters from a function declared inside a json object', () => {
    const obj = {
      fn(param1, param2) {
        return param1 + param2;
      },
    };

    const metadata = reflect(obj.fn);
    expect(metadata.parameters).toHaveLength(2);
  });

[ktutnik]

Do you have another case that is failing, so I can add more fixes at once?

[sebaplaza]

Not for now, i detected these cases running your fixes against production code.

Then i wrote the tests to reproduce the case.

Just for information: I was using this library (abandoned) before migrate to yours.

I found these cases where plumier is not working, but plumier is giving a lot more of metadata information than the others.

[ktutnik]

A quick trace, about your last issue:

Its difficult for Reflect to parse the function, because if you pass it like that it becomes an invalid javascript syntax.

const obj = {
      fn(param1, param2) {
        return param1 + param2;
      },
};

// if you pass it like  obj.fn 
// the resulting javascript syntax is like below, which is invalid javascript syntax

fn(param1, param2) {
    return param1 + param2;
}

I agree, logically Reflect should be able to reflect function like above, but I still have no idea how to fix above issue, but as a workaround, you can use lambda function or inline function like below.

const obj = {
      fn: (param1, param2)  => {
        return param1 + param2;
      },
};

[sebaplaza]

oh i see, you're right, if we isolate the function this is not a valid syntax.

Maybe is just a bug that this code can handle this case.

It's a bit curious, but in my example, the result of reflect() is detecting a function

{ 
  "kind": "Function", 
  "name": "fn", 
  "parameters": [] 
}

[ktutnik]

I see, it uses regex and doesn't require a valid javascript syntax. We was using regex in our earlier version and found its getting more issue most case.

I think I can mix between the current logic with regex to get a more robust result. Will give you an update about this.

[sebaplaza]

Hello @ktutnik ,

do you have any updates about this issue ?

do you think it could be resolved or i should find another way to achieve it ?

Thanks !

[ktutnik]

HI @sebaplaza sorry for the delay, I fixed the issue and created a canary release for you, go for @plumier/reflect@1.1.1-canary.0 and let me know if you found another issue.

Please close this thread if the issue is fixed, prefer a new one if you found another issue.

About your DI project, is that an open source or a private project? Is it possible for me to see it?

Thank you.

[sebaplaza]

Thanks a lot @ktutnik, the last case is working now.

I'm having a last case, with async functions inside an object.

  it('should returns parameters from a async function declared inside a json object', () => {
    const obj = {
      async fn(param1, param2) {},
    };

    const metadata = reflect(obj.fn);
    expect(metadata.parameters).toHaveLength(2);
  });

[ktutnik]

lol, I really didn't noticed about async function. Fixed by #1033

please try again now @plumier/reflect@1.1.1-canary.1

[sebaplaza]

Everything seems to work now !

Thanks !

[ktutnik]

happy to help. im closing this thread now, just open another issue if you found one.