Skip to content

Error using before transforms on files with decorators and class constructor parameter #17551

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
filipesilva opened this issue Aug 1, 2017 · 8 comments
Assignees
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API

Comments

@filipesilva
Copy link
Contributor

Description of the bug:

When using any before transform on a file containing a class with both:

  • a decorator
  • a constructor parameter with a class type

An error will be thrown (TypeError: Cannot read property 'exports' of undefined at resolveNameHelper).

This error goes away if:

  • the transform is removed
  • the transform is moved from before to after
  • the decorator is removed
  • the constructor type is changed from a class to a basic type (boolean, number, etc).

TypeScript Version: 2.4.2

Code

function classDecorator<T extends { new(...args: any[]): {} }>(constructor: T) {
  return class extends constructor { }
}

class SomeClass {}

@classDecorator
export class AppComponent {
  constructor(test: SomeClass) { }
}

'change me';

Reproduction of this bug requires using the transforms API.

I have prepared a repository with a simple reproduction that runs this file: https://github.com/filipesilva/ts-transform-decorator-bug/blob/master/test.ts

git clone https://github.com/filipesilva/ts-transform-decorator-bug
cd ts-transform-decorator-bug
npm install
npm test

Expected behavior:
File is emitted without errors.

Actual behavior:
An error is thrown:

D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:27546
                        var moduleExports = getSymbolOfNode(location).exports;
                                                                     ^
TypeError: Cannot read property 'exports' of undefined
    at resolveNameHelper (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:27546:70)
    at resolveName (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:27491:20)
    at resolveEntityName (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:28152:26)
    at Object.getTypeReferenceSerializationKind (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:47726:31)
    at serializeTypeReferenceNode (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:52219:30)
    at serializeTypeNode (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:52167:28)
    at serializeTypeOfNode (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:52039:28)
    at serializeParameterTypesOfNode (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:52074:42)
    at addOldTypeMetadata (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:51962:98)
    at addTypeMetadata (D:\sandbox\ts-transform-decorator-bug\node_modules\typescript\lib\typescript.js:51953:17)

/cc @rbuckton

@DanielRosenwasser DanielRosenwasser added API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API labels Aug 1, 2017
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 2.5 milestone Aug 1, 2017
@arciisine
Copy link

I'm actually running into a very similar problem. I am currently attempting to decorate fields of a class while using transpilation. By default the code is breaking on the fact that the PropertyDeclarations do not have a parent property set generating new elements. Using the built in visitor, the parent relationship seems to be severed. Additionally, if I do add the parent relationship in manually, I am failing in the exact same way above, that the symbol information is not updated or created when using the visitor pattern. The createNode/updateNode flow leaves me with a SourceFile object and no symbol data.

@arciisine
Copy link

Attaching a gist that is able to reproduce both issues I mentioned above https://gist.github.com/arciisine/2ba754f561d3e28b138aeed686a800d3

@clydin
Copy link
Contributor

clydin commented Aug 15, 2017

Did some investigation into the issue and it seems that the underlying cause is surfaced (in this case) by enabling the emit decorator metadata compiler option. When the source file node is updated, the symbol is removed from the node. The TS transformer tries to use the symbol to generate the type information for the decorator metadata but the symbol is no longer accessible from the node. The call into the emit resolver that throws is https://github.com/Microsoft/TypeScript/blob/master/src/compiler/transformers/ts.ts#L1875

@clydin
Copy link
Contributor

clydin commented Aug 15, 2017

Also, haven't done any additional tests but similar issues may arise in other areas that assume a node has a symbol.

@tbosch
Copy link

tbosch commented Aug 15, 2017

This seems related to #17384. Maybe the workaround there also works here.

@clydin
Copy link
Contributor

clydin commented Aug 15, 2017

An interim fix could be to get the original node within getSymbolOfNode before accessing the symbol. (Preliminary test performed and the change appears to work.)
However, there is a more systemic issue at play. Transformer alterations may render the symbol information invalid. A basic (and benign) example of this is demonstrated in issue #17552. Unused imports are not elided due to the symbol still reporting that it is referenced. A mechanism to allow a transformer to force type analysis or notify the compiler that it is needed after transformer completion (or both) would be useful in this scenario. The above interim fix could then be altered to walk up the origin hierarchy to find the most recent symbol.

@arciisine
Copy link

@tbosch thanks for the pointer. I integrated the tsickle transformers into my workflow, and it resolved the code errors I was seeing. Though it seems to have no impact on import eliding, as I'm losing the import my identifier is referencing.

@filipesilva
Copy link
Contributor Author

Awesome, thanks for the fix @rbuckton !

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API Relates to the public API for TypeScript Bug A bug in TypeScript Domain: Transforms Relates to the public transform API
Projects
None yet
Development

No branches or pull requests

6 participants