Skip to content
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

compiler-cli: lowering_expression transformer does not handle forwardRef like function #20216

Closed
shlomiassaf opened this issue Nov 6, 2017 · 16 comments
Labels
area: compiler Issues related to `ngc`, Angular's template compiler freq2: medium type: bug/fix
Milestone

Comments

@shlomiassaf
Copy link
Contributor

shlomiassaf commented Nov 6, 2017

@Component({
  selector: 'app-test-component',
  templateUrl: './test-component.component.html',
  styleUrls: ['./test-component.component.css']
})
export class TestComponentComponent {
  prop: string;
}

export function MyPropDecorator(value: any) {
  return (target: Object, key: string) => {  }
}

export class MyClass {
  @MyPropDecorator(() => TestComponentComponent)
  prop: string;
}
SHLASSAF-M-4099:testproj shlomiassaf$ ng build --aot --build-optimizer
Date: 2017-11-06T04:02:00.134Z                                                     
Hash: 957bab95fbd81c3b3b0f
Time: 3658ms
chunk {inline} inline.bundle.js, inline.bundle.js.map (inline) 5.83 kB [entry] [rendered]
chunk {main} main.bundle.js, main.bundle.js.map (main) 303 bytes [initial] [rendered]
chunk {polyfills} polyfills.bundle.js, polyfills.bundle.js.map (polyfills) 323 bytes [initial] [rendered]
chunk {styles} styles.bundle.js, styles.bundle.js.map (styles) 11.3 kB [initial] [rendered]

ERROR in Error: TypeError: Cannot read property 'kind' of undefined
    at nodeCanBeDecorated (/Users/shlomiassaf/Desktop/Cisco/personal/testproj/node_modules/typescript/lib/typescript.js:7805:35)

Its not @angualr-devkit/build-optimizer, i removed it from the command line, still errors.

This will work:

export function MyPropDecorator(value: any) {
  return (target: Object, key: string) => {  }
}

export class MyClass {
  @MyPropDecorator(TestComponentComponent)
  prop: string;
}

Being explicit won't help

export function MyPropDecorator(value: () => typeof TestComponentComponent) {
  return (target: Object, key: string) => {  }
}

export class MyClass {
  @MyPropDecorator(() => TestComponentComponent)
  prop: string;
}

BUT, being static will help:

export function MyPropDecorator(value: () => typeof TestComponentComponent) {
  return (target: Object, key: string) => {  }
}

export function MyGetter() {
  return TestComponentComponent;
}

export class MyClass {
  @MyPropDecorator(MyGetter)
  prop: string;
}

Thanks!

Angular CLI: 1.5.0
Node: 8.4.0
OS: darwin x64
Angular: 5.0.0
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

@angular/cli: 1.5.0
@angular-devkit/build-optimizer: 0.0.32
@angular-devkit/core: 0.0.20
@angular-devkit/schematics: 0.0.35
@ngtools/json-schema: 1.1.0
@ngtools/webpack: 1.8.0
@schematics/angular: 0.1.0
typescript: 2.4.2
webpack: 3.8.1
@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Nov 6, 2017

cc @tbosch @chuckjaz

@tbosch I think its an edge case of microsoft/TypeScript#17384 you didn't handle

Also, using TS 2.5.3 and 2.6.1 did not solve the problem.

@shlomiassaf
Copy link
Contributor Author

Another update to help identify the source:

export class MyClass {
  @MyPropDecorator(() => 15)
  prop: string;
}

It doesn't have to be a custom type, even intrinsic types will fail.

shlomiassaf added a commit to shlomiassaf/TypeScript that referenced this issue Nov 6, 2017
When calling `setOriginalNode` attach the original node's parent to the node, if the node does not have a parent.

This should solve all `TypeError: Cannot read property 'kind' of undefined` errors when working with transformers and decorators.

See microsoft#17384
Also see angular/angular#20216

Query:
The implementation does not invalidate `node.parent` when `original` is undefined.
I'm not sure if this is a valid use case but based on the existing logic, `node.original` is invalidated even if `original` is undefined...
@shlomiassaf
Copy link
Contributor Author

shlomiassaf commented Nov 6, 2017

OK, for now, if by any change someone is interested, this is a working, monkey patching, workaround:

/**
 * This module will patch the `@angular/compiler-cli` so it will correctly lower expression to declarations in decorators.
 * See https://github.com/angular/angular/issues/20216
 */
import * as ts from 'typescript';
import '@angular/compiler-cli';
const lowerExpressions = require('@angular/compiler-cli/src/transformers/lower_expressions');

function touchNode(node: ts.Node) {
  if (!node.parent) {
    const original: ts.Node = <any> ts.getOriginalNode(node);
    if (original !== node && original.parent) {
      node.parent = original.parent;
      ts.forEachChild(node, touchNode)
    }
  }
}

const getExpressionLoweringTransformFactory = lowerExpressions.getExpressionLoweringTransformFactory;
lowerExpressions.getExpressionLoweringTransformFactory = function(requestsMap, program) {
  const fn = getExpressionLoweringTransformFactory(requestsMap, program);
  return context => sourceFile => {
    const result = fn(context)(sourceFile);
    if (result !== sourceFile) {
      ts.forEachChild(result, touchNode)
    }
    return result;
  };
};

just import this file in the top of your webpack config module, or if you use ngc invoke it from a local file.

@kirillgroshkov
Copy link

waiting for the fix..

@simonalbrecht
Copy link

Any tips on how to integrate this fix into an @angular/cli@1.5.0 project without having to ng eject?

@shlomiassaf
Copy link
Contributor Author

create a local ng executable that acts as a pass-through.
It will run the workaround first, then pass control to the original ng script.

Assuming the following file is in the root of the project:

Filename: ng

#!/usr/bin/env node

/// WORKAROUND HERE

require('node_modules/.bin/ng');

Did not test it, but should work.
Maybe some modifications based on OS.

You can also add it to your package.json scripts

@niveo
Copy link

niveo commented Nov 22, 2017

@niveo
Copy link

niveo commented Nov 28, 2017

typestack/class-transformer#108

@rmrevin

export function serializeType<T>(object: T) {
  return function () { return object; }
}

export class CatalogItem {

  id: string;

  @Type(serializeType(CatalogCategory))
  category?: CatalogCategory = null;

  @Type(serializeType(PackingVariant))
  packing: PackingVariant;

  price: string = null;

}

@niveo
Copy link

niveo commented Dec 15, 2017

angular/angular-cli#8434

@gmavritsakis

I had the same issue with:
Angular CLI 1.6.1
Typescript 2.4.2
Angular 5.0.2

Found a solution by changing typescript.js for now.
Replace all the function
function nodeCanBeDecorated(node)
with the following code

function nodeCanBeDecorated(node) {
       switch (node.kind) {
           case 229 /* ClassDeclaration */:
               // classes are valid targets
               return true;
           case 149 /* PropertyDeclaration */:
               // property declarations are valid if their parent is a class declaration.
   			// return node.parent.kind === 229 /* ClassDeclaration */;
   			return (node.parent && node.parent.kind === 229) || (node.original && node.original.parent && node.original.parent.kind === 229);
           case 153 /* GetAccessor */:
           case 154 /* SetAccessor */:
           case 151 /* MethodDeclaration */:
               // if this method has a body and its parent is a class declaration, this is a valid target.
               return node.body !== undefined &&
   				// && node.parent.kind === 229 /* ClassDeclaration */;
   				(node.parent && node.parent.kind === 229) || (node.original && node.original.parent && node.original.parent.kind === 229);
           case 146 /* Parameter */:
               // if the parameter's parent has a body and its grandparent is a class declaration, this is a valid target;
               // return node.parent.body !== undefined
               //     && (node.parent.kind === 152 /* Constructor */
               //         || node.parent.kind === 151 /* MethodDeclaration */
               //         || node.parent.kind === 154 /* SetAccessor */)
   			//     && node.parent.parent.kind === 229 /* ClassDeclaration */;
   			
   			var parent = node.parent || (node.original && node.original.parent);
   			return parent && parent.body !== undefined &&
                     (parent.kind === 152
                        || parent.kind === 151
                        || parent.kind === 154) && parent.parent.kind === 229;
       }
       return false;
   }

Which comes from here, if you compile typescript:
shlomiassaf/TypeScript@7017fa2

@kirillgroshkov
Copy link

I want to share my workaround that I put together after reading other advices here and there. It's patching an ng script from @angular/cli@1.6.4 (current version at the moment of writing). Because of that it prevents updating @angular/cli, so be prepared.

  1. Put a patched copy of ng somewhere in your repo. You can grab patched version from here:
    https://github.com/kirillgroshkov/angular-cli/blob/18f14d71bc5d73dba488f8fc1d08fd46d1d885f1/packages/%40angular/cli/bin/ng

Here's what changed, compared to the original file: https://github.com/kirillgroshkov/angular-cli/commit/18f14d71bc5d73dba488f8fc1d08fd46d1d885f1

  1. Add a postinstall script in your package.json that will run every time after you do npm install (or yarn install) and copy/overwrite ng from your location to node_modules/@angular/cli/bin/ng.
"postinstall": "echo 'patching @angular/cli' & cp ./other/ng ./node_modules/@angular/cli/bin"
  1. Run your build as normal, you should see console.log message "!!! using patched @angular/cli".
  2. Profit

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@daixtrose
Copy link

@kirillgroshkov Your solution cannot be applied in my context (@angular 5.0.3)

$ ls node_modules/@angular
common/  compiler/  compiler-cli/  core/  forms/  http/  platform-browser/  
platform-browser-dynamic/

$ ls node_modules/@angular/compiler-cli
index.d.ts  index.js  index.js.map  ngtools2.d.ts  ngtools2.js  ngtools2.js.map  
package.json  README.md  src/

any hints?

@kirillgroshkov
Copy link

@daixtrose My solution is when @angular/cli is used in the project. Seems you you don't use it (cause I can't see it in node_modules), then how do you compile your angular project? Anyway, if you dig deeper, you'll find what lines you need to add to your compiler script to make it work, good luck!

ps: Lines are there, between 4 and 29:
https://github.com/kirillgroshkov/angular-cli/blob/18f14d71bc5d73dba488f8fc1d08fd46d1d885f1/packages/%40angular/cli/bin/ng

@ngbot ngbot bot modified the milestones: Backlog, needsTriage Feb 26, 2018
@splincode
Copy link
Contributor

@splincode
Copy link
Contributor

Also not working in Angular

class Test {

@Transform(
      (value) => plainToClass(ApplicationData, JSON.parse(value)),
      { toClassOnly: true }
  )
  @Transform(
      (value) => JSON.stringify(classToPlain(value)),
      { toPlainOnly: true }
  )
  data: ApplicationData;

}

@JoostK JoostK added area: compiler Issues related to `ngc`, Angular's template compiler and removed area: core Issues related to the framework runtime labels Mar 15, 2020
@JoostK
Copy link
Member

JoostK commented Mar 15, 2020

Apparently, this started working from Angular CLI 1.7 onwards, so somehow the CLI was involved. I can't reproduce this in newer versions of Angular (or even with Angular 5.0.0, TS 2.4.2 and CLI 1.7.0) so I'm closing as resolved.

@JoostK JoostK closed this as completed Mar 15, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: compiler Issues related to `ngc`, Angular's template compiler freq2: medium type: bug/fix
Projects
None yet
Development

No branches or pull requests

9 participants