Skip to content

emitDecoratorMetadata should emit type resolvers #41201

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

Closed
paeolo opened this issue Oct 22, 2020 · 3 comments
Closed

emitDecoratorMetadata should emit type resolvers #41201

paeolo opened this issue Oct 22, 2020 · 3 comments
Labels
Duplicate An existing issue was already created

Comments

@paeolo
Copy link

paeolo commented Oct 22, 2020

TypeScript Version: 4.1.0-dev.20201022

Search Terms: emitDecoratorMetadata, decorators, design:type, type, resolver, reflection

Code:

Suppose we are doing Node.js code and have a plain old circular dependency:

classOne.ts

import { property } from './utils';
import ClassTwo from './classTwo';

export default class ClassOne {
  @property
  property: ClassTwo;
}

classTwo.ts

import { property } from './utils';
import ClassOne from './classOne';

export default class ClassTwo {
  @property
  property: ClassOne;
}

and suppose we want to bring a bit of reflection in our code:

index.ts

import 'reflect-metadata';
import { getDesignType } from './utils';
import ClassOne from './classOne';
import ClassTwo from './classTwo';

console.log(`Hello World! ${ClassOne.name} and ${ClassTwo.name};)`);

console.log(`--`);

console.log(`${ClassOne.name} property is of type ${getDesignType(ClassOne, 'property')}`);
console.log(`${ClassTwo.name} property is of type ${getDesignType(ClassTwo, 'property')}`);

utils.ts

/**
 * Use typescript emited metadata to get the type
 *
 * @param ctor
 * @param key
 */
export const getDesignType = (ctor: Function, key: string) => {
  const type = Reflect.getMetadata('design:type', ctor.prototype, key);
  return type?.name;
}

/**
 * Dummy decorator
 *
 * @param target
 * @param key
 */
export const property = (target: any, key: string) => { }

Expected behavior:

foo@bar:~$ node ./dist
Hello World! ClassOne and ClassTwo;)
--
ClassOne property is of type ClassTwo
ClassTwo property is of type ClassOne

Actual behavior:

foo@bar:~$ node ./dist
Hello World! ClassOne and ClassTwo;)
--
ClassOne property is of type ClassTwo
ClassTwo property is of type undefined

Explanation:

Well it's not much a surprise, we are running into a circular import issue:

  • index.ts is importing ClassOne
  • ClassOne is importing ClassTwo
  • ClassTwo is importing ClassOne
  • Since ClassOne is being read, return the cached value of ClassOne
  • __metadata("design:type", classOne_1.default) is evaluated
  • classOne_1.default is evaluated to undefined
  • ...

Workaround:

There is a simple workaround and it's already used by many libraries but unfortunately not by typescript. The idea is not to emit the type but to emit a type resolver ie. instead of emitting ClassFoo one should emit () => ClassFoo. This way we can evaluate the type resolver in index.ts at a time where ClassOne import is already done.

Playground Link:

https://github.com/paeolo/typescript-emit-resolver

Related Issues:

This issue is the root of many issues. Solving this will enable a much better code where reflection is used by not introducing the type resolver as a redondancy.

Please note that trying to solve the circular import is not a good solution. Sometimes circular import are inherents and it's very hard to both focus on code and trying to solve every "circular import" related problems. tslib would benefit a lot from being more resilient to such issues.

@MartinJohns
Copy link
Contributor

Kinda a duplicate of #27519, tho the behavior is different.

FYI, the TypeScript team expressed several times that they do not intent to further develop any decorator feature until the TC39 proposal is finished.

@paeolo
Copy link
Author

paeolo commented Oct 23, 2020

@MartinJohns Indeed it's a duplicate of #27519.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants