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

Add INPUT/OUTPUT as properties to Ng2MapComponent #64

Closed
wants to merge 1 commit into from

Conversation

riltsken
Copy link
Contributor

When using Ionic2 RC4 (currently latest version as of Dec 29 2016) the ngc compiler complains about the INPUT / OUTPUT dynamic properties on the Ng2MapComponent class. Adding these as properties fixes the issue.

Wasn't sure if you wanted me to include the compiled files in my PR or not. I can open another PR that has everything generated if you would like.

Error when using latest ionic2 RC4

[15:17:20]  Error at 
            /home/sam/work/nibbler/.tmp/node_modules/ng2-map/dist/components/ng2-map.component.ngfactory.ts:144:20 
[15:17:20]  Property 'backgroundColor' does not exist on type 'Ng2MapComponent'. 
...
[15:17:20]  Error at 
            .tmp/node_modules/ng2-map/dist/components/ng2-map.component.ngfactory.ts:400:20 
[15:17:20]  Property 'options' does not exist on type 'Ng2MapComponent'. 
[15:17:20]  ngc failed 
[15:17:20]  ionic-app-script task: "build" 

When using Ionic2 RC4 (currently latest version as of Dec 29 2016) the ngc compiler complains about the INPUT / OUTPUT dynamic properties on the Ng2MapComponent class. Adding these as properties fixes the issue.
@allenhwkim
Copy link
Contributor

This doesn't look DRY

@riltsken
Copy link
Contributor Author

I can attempt to extract into an interface and see if I can use the interface properties for the input and output decorator as well.

@riltsken
Copy link
Contributor Author

riltsken commented Jan 2, 2017

Spent a couple hours trying to figure out a way to create the INPUT/OUTPUT properties dynamically but couldn't come up with any. Not sure how to fix this without avoiding duplication.

@allenhwkim
Copy link
Contributor

allenhwkim commented Jan 2, 2017

Are you using old version of ng2-map.
The recent version does not have this file dist/components/ng2-map.component.ngfactory.ts, but it has ng2-map.component.metadata.json

@riltsken
Copy link
Contributor Author

riltsken commented Jan 2, 2017

We just switched to the most recent version (after being on a fork for a little bit since generated .d.ts files were missing before). ng2-map.component.ngfactory.ts gets generated when we run compilation of our project when using this library. Not sure how accurate this blog post is, but it suggests that this is normal behavior https://medium.com/@isaacplmann/getting-your-angular-2-library-ready-for-aot-90d1347bcad#.maojk4pzs.

  1. Don’t include *.ngFactory.ts files in the npm package
    Consumers of the library will generate these on their own. Add to .npmignore:
    /compiled

or

*.ngFactory.ts

or no change needed if you already have

*.ts

Admittedly I don't know enough about angular internals to understand the reasoning for this to happen when we start using the library. I just know that when using INPUTS and OUTPUTS without defining members causes ngc to barf on our project because these attributes are missing from the class instance but still being created.

I don't expect action to be taken at this point though unless others are encountering this issue. We can live off a fork for now with these added to our project and move on until next time we update (maybe we'll have learned enough on how to fix this or it will be more obvious).

@allenhwkim
Copy link
Contributor

Closing it until we find a solution for DRY solution

@allenhwkim allenhwkim closed this Jan 17, 2017
@allenhwkim allenhwkim reopened this Jan 19, 2017
@allenhwkim
Copy link
Contributor

It seems this only happens for aot compile. I was not able to succeed in generating ng2-map.component.ngfactory.ts

Could you post the file here, so that I can take a look at it?

@riltsken
Copy link
Contributor Author

riltsken commented Jan 20, 2017

Here is the file: https://gist.github.com/riltsken/a3d12b409ea8346e6f8d85589dd9c223

You can generate it yourself locally if you change skipTemplateCodegen from true to false in tsconfig.ngc.json and then npm run build

@allenhwkim
Copy link
Contributor

Do you have any pre-compiler script for this? If you have one, I can use it for building process so that I can keep source code DRY, but dist/****.ts is very detailed.

@riltsken
Copy link
Contributor Author

No we are just working off a fork right now with the changes in this PR. When we run our full build normally it will create this file for us at that point in time.

@allenhwkim
Copy link
Contributor

When using Ionic2 RC4, does it look for source code from 'src/.ts', or from 'dist/.js' as described from package.json?

@allenhwkim
Copy link
Contributor

allenhwkim commented Jan 21, 2017

@riltsken I have released 0.14.3 with npm beta tag, not a latest release.
Can you verify your ionic compile compilation with that module?

allenhwkim added a commit that referenced this pull request Jan 23, 2017
@allenhwkim allenhwkim closed this Jan 23, 2017
@riltsken
Copy link
Contributor Author

Don't want to leave you hanging but did want to check in. We upgraded to ionic RC5 and ionic-app-scripts 1.0.0 which changed some webpack configuration. When I use official 0.14.1 (not the 0.14.3 beta you mention here) there are no build errors, but haven't had a chance to dig in further to verify everything is working (that factory file actually doesnt even get generated anymore so I am suspicious.)

@riltsken
Copy link
Contributor Author

riltsken commented Jan 23, 2017

Now that I think about it though webpack might be irrelevant to the conversation since it is ngc that was complaining and ngc didn't complain after I described the above.

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

Successfully merging this pull request may close these issues.

2 participants