Hookyns / tst-reflect

Advanced TypeScript runtime reflection system
MIT License
338 stars 11 forks source link

Type Forwarding #43

Closed dhkatz closed 2 years ago

dhkatz commented 2 years ago

Is there a way to somehow hint that a type parameter should be forwarded because right now it doesn't seem to work?

/**
 * @reflect
 */
function bar<TType>(name: string, description: string) {
  const typeInfo = getType<TType>()

  console.log(typeInfo.name) // Logs 'TType' instead of class name

  return (target, key, descriptor) => {
    return descriptor
  }
}

/**
 * @reflect
 */
function foo<TType>(name: string) {
  return bar<TType>(name, 'description')
}

class Test {
  @foo('test')
  public fetch() {
  }
}

This also seems to print a warning where the decorator foo is used:

[WRN] tst-reflect-transformer: Unexpected decorator argument. Only constant values are allowed.

If I call @bar('test', 'description') directly instead it works fine and I don't get the warning


I looked at the generated JS and the issue seems to be that types aren't forwarded when expected:

/**
 * @reflect
 */
function bar(name, description) {
  var __genericParams__ = arguments[--arguments.length];
    delete arguments[arguments.length];
    const typeInfo = (__genericParams__ && __genericParams__.TType);
    // ...
}

/**
  * @reflect
  */
function foo(name) {
  var __genericParams__ = arguments[--arguments.length]; 
  delete arguments[arguments.length]; 
  return bar(name, 'description', { TType: _ßr.Type.store.get(30244) }); // <-- The issue is here
  // I would expect to see something like:
  // return bar(name, 'description', { TType: __genericParams__.TType });
}

I tried looking through examples that were in the repo but I can't really find anything. This seems a bit unintuitive right now without forwarding

Hookyns commented 2 years ago

Hello @dhkatz, I think that's because I didn't want to allow this cuz of performance, but I think I know how to do it now. With @reflect there should be no perf issue.

dhkatz commented 2 years ago

Thank you for taking a look into this 🙂

This is probably a breaking change so I understand your hesitance 😅

Hookyns commented 2 years ago

Done.

Try new versions tst-reflect@0.7.5 & tst-reflect-transformer@0.9.10.

dhkatz commented 2 years ago

Thank you very much 😀

Hookyns commented 2 years ago

@all-contributors add @dhkatz for bug report.

allcontributors[bot] commented 2 years ago

@Hookyns

I've put up a pull request to add @dhkatz! :tada: