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

AOT Compilation with Angular CLI #514

Closed
timmyrosen opened this issue Sep 22, 2017 · 13 comments · Fixed by #516
Closed

AOT Compilation with Angular CLI #514

timmyrosen opened this issue Sep 22, 2017 · 13 comments · Fixed by #516

Comments

@timmyrosen
Copy link

I'm submitting a...


[x] Bug
[ ] Feature Request / Proposal
[ ] Question

I'm using...


NG Dynamic Forms Version: 1.4.31

[ ] Basic UI
[ ] Bootstrap UI  
[ ] Foundation UI
[ ] Ionic UI
[ ] Kendo UI
[x] Material  
[ ] NG Bootstrap
[ ] Prime NG

Description

Hi!
Awesome plugin for starters!

I'm experiencing a problem when compiling with aot flag in Angular CLI (v 1.4.1) (compiler-cli v 4.2.4). The problem does not appear when using --aot=false

When trying to add a DynamicInputModel to a DynamicFormGroupModel I get the following error in console:

ERROR TypeError: t.splice is not a function
    at 3.37cf5c36cf41307ab3e1.chunk.js:1
    at Array.forEach (<anonymous>)
    at e.insertFormGroupControl (3.37cf5c36cf41307ab3e1.chunk.js:1)
    at e.addFormGroupControl (3.37cf5c36cf41307ab3e1.chunk.js:1)
    at e.dropInput (3.37cf5c36cf41307ab3e1.chunk.js:1)
    at Object.handleEvent (3.37cf5c36cf41307ab3e1.chunk.js:1)
    at Object._ [as handleEvent] (vendor.fcadd8823737b1670225.bundle.js:1)
    at Object.handleEvent (vendor.fcadd8823737b1670225.bundle.js:1)
    at oe (vendor.fcadd8823737b1670225.bundle.js:1)
    at vendor.fcadd8823737b1670225.bundle.js:1

As I said, it works perfectly locally and when --aot=false so I don't think there is anything wrong with my code.

Thanks!

@udos86
Copy link
Owner

udos86 commented Sep 22, 2017

@timmyrosen Hi!

Awesome plugin for starters!

Thank you very much!

As I said, it works perfectly locally and when --aot=false so I don't think there is anything wrong with my code.

I can confirm, this is a bug indeed. I'll deliver a patch as soon as possible.

Thanks for reporting this.

@udos86
Copy link
Owner

udos86 commented Sep 22, 2017

@timmyrosen

After some further investigation I was able to locate the origin of this bug.

Angular CLI just breaks the instanceof operator.

This is the original library source code:

if (groupModel instanceof DynamicFormGroupModel) {
  groupModel.insert(index, controlModel);

} else {
  (groupModel as DynamicFormControlModel[]).splice(index, 0, controlModel);
}

That's the output of Angular CLI ng build --aot:

if (groupModel instanceof __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b" /* DynamicFormGroupModel */]) {
  groupModel.insert(index, controlModel);
}
else {
  groupModel.splice(index, 0, controlModel);
}

I'll file a bug.

@timmyrosen
Copy link
Author

timmyrosen commented Sep 22, 2017

@udos86

I see, thanks for quick reply!

So this is in the hands of the Angular CLI team now?

@udos86
Copy link
Owner

udos86 commented Sep 22, 2017

@timmyrosen Yep!

angular/angular-cli#7788.

@about-code
Copy link

about-code commented Sep 24, 2017

@udos86 Are you sure its a CLI-problem? I don't think the replacement with the webpack variable itself does break the instanceof operator, given that __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b"] references the correct constructor function. If it didn't it were a severe webpack problem causing many other projects to fail. I don't think that's the case here.

E.g. you may want to try the following snippet in Chrome:

class Foo {
    constructor() {}
}

class Bar {
    constructor() {}
}

__WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__ = {
 "b": Foo,
 "c": Bar   
};

var foo = new Foo();

console.log(foo instanceof __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b" /* DynamicFormGroupModel */]);
console.log(foo instanceof __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["c" /* DynamicFormGroupModel */]);

// Output:
// true
// false

I think the problem is that in your code-samples there's the assumption that groupModel is always an array if it is not an atomic instance. But if its null, for example, or an atomic instance of a completely different class, then its neither an instance of DynamicFormControlModel nor DynamicFormControlModel[].

@udos86
Copy link
Owner

udos86 commented Sep 24, 2017

@about-code Hi there, old coding friend! Thanks for your contribution to this issue!

If it didn't it were a severe webpack problem causing many other projects to fail. I don't think that's the case here.

I'm aware that this may seem highly unlikely , but here's a screenshot to beat the odds:

bildschirmfoto 2017-09-24 um 13 13 21

As you can see groupModel is indeed a DynamicFormGroupModel.

Yet the instanceof check returns false as result.

I think the problem is that in your code-samples there's the assumption that groupModel is always an array if it is not an atomic instance. But if its null, for example, or an atomic instance of a completely different class, then its neither an instance of DynamicFormControlModel nor DynamicFormControlModel[].

Technically speaking, you're right. But due to the function parameter typing this case is actually forbidden:

insertFormGroupControl(index: number,
    formGroup: FormGroup,
    groupModel: DynamicFormControlModel[] | DynamicFormGroupModel,
...controlModels: DynamicFormControlModel[]):

Given the fact that the code works flawless when leaving of --aot flag, my only conclusion at the moment can be to blame CLI / Webpack for all this.

@udos86 udos86 added the webpack label Sep 24, 2017
@about-code
Copy link

In the console output it looks as if __WEBPACK... variable holds a reference on a DynamicFormGroupModel constructor and groupModel is an instance of DynamicFormGroupModel. So the instanceof check should indeed return true.

What happens if you compare

groupModel.constructor === __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b"]

If the result is false it is a sign that there likely exist two different definitions of DynamicFormGroupModel constructor function in the bundles for whatever reason. In your screenshot on the left there are multiple bundles loaded. May it be that webpack put another definition of the class in one of the other bundles?

Is there another webpack config apart from webpack.config.js? You don't use CommonsChunkPlugin there.

@udos86
Copy link
Owner

udos86 commented Sep 24, 2017

@about-code

If the result is false it is a sign that there likely exist two different definitions of DynamicFormGroupModel constructor function in the bundles for whatever reason.

Yep, that's exactly my current assumption according to this console output:

groupModel.constructor === __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b"]
false
groupModel.constructor.name === __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b"].name
true

Is there another webpack config apart from webpack.config.js? You don't use CommonsChunkPlugin there.

No, Angular CLI is hiding away Webpack entirely from the developer. There's no way of manipulating it's configuration. That's why my hands are pretty much tied here.

@udos86
Copy link
Owner

udos86 commented Sep 24, 2017

@about-code

Alright, here is the ultimate evidence, what's going on.

This is the app code that creates the groupModel instance:

import { DynamicFormGroupModel } from "@ng-dynamic-forms/core";

new __WEBPACK_IMPORTED_MODULE_0__ng_dynamic_forms_core__["k" /* DynamicFormGroupModel */]({...})

And that's the library code that uses instanceof:

import { DynamicFormGroupModel } from "../model/form-group/dynamic-form-group.model";

if (groupModel instanceof __WEBPACK_IMPORTED_MODULE_2__model_form_group_dynamic_form_group_model__["b" /* DynamicFormGroupModel */]) {
  groupModel.insert(index, controlModel);
}

It's obvious...but what's the reason for this bundling error?

@about-code
Copy link

@udos86 Have seen the thread over at angular-cli. If you want to give ng-packagr a try you might also find a script in my repo helpful. It iterates over your packages folder and builds every package with an ng-package.json.

You might need to adjust the relative path in the script pointing to '../packages', depending on where you put the script in your repo. If you have any third-party deps you need to add them to the externals section of the respective package's ng-package.json. Then run the script as a node script (requires ng-packagr and rimraf).

@udos86
Copy link
Owner

udos86 commented Sep 25, 2017

@about-code

You know, I really like your work and I'm following your progress on your new project, of course.

And I also like ng-packagr - it's a really nice tool. But with this project I wanted to succeed on my own. So after spending multiple hours tonight on this I finally got it going.

From the next release on NG Dynamic Forms will meet Angular Package Format requirements which ultimately resolves this issue here.

Maybe you'd like to have a look at 5e53e64

What's really important is that your tsconfig.json for ngc is properly configured as well as your typings property in package.json.

This took me quite a while to get my head around.

Thanks @timmyrosen for bringing this up and @about-code for your expertise.

@udos86 udos86 mentioned this issue Sep 25, 2017
@timmyrosen
Copy link
Author

Wow, great work! Will test tomorrow, glad I could help!

@about-code
Copy link

You know, I really like your work and I'm following your progress on your new project, of course.

So do I.

But with this project I wanted to succeed on my own

I like that attitude. Good to know where I can ask for advice should I have to find out the hard way, that my lazyness hasn't teached me the lessons you've just learned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants