search

mariuszfoltak / angular2-datatable

DataTable - Simple table component with sorting and pagination for Angular2
202 stars 182 forks source link

Removed AOT compilation incompatible "private" visibility modifiers #44

Closed taguan closed 8 years ago

taguan commented 8 years ago

Removed private on @Input fields and fields used in templates so typescript compilation after AOT compilation doesn't emit errors anymore

Example of such an error :

error TS2341: Property 'sortBy' is private and only accessible within class 'DefaultSorter'.

Tuupertunut commented 8 years ago

Yeah, I got those errors some time ago when I was updating another fork of this project, but suddenly when moving to this fork, the AOT compiler didn't complain about private modifiers anymore. It was compiling on my machine just fine and also on the travis CI of this project (as you can see for yourself).

So are you getting these errors now or is this just future proofing?

You also seemed to remove the caret from rxjs version, probably because it was giving peer dependency warnings. My thoughts are that forcing rxjs to version 5.0.0-beta.12 may cause this project to fail on future angular updates. That is because the declaration @angular/... ^2.1.0 allows angular to update but not rxjs. At some point in the future angular might require a newer version of rxjs.

I'm not absolutely sure about this, but I think letting rxjs dependency be a bit ahead of time is safer than leaving it behind time, even if that causes peer dependency warnings. Libraries often have backwards compatibility (new library works for old requirements) but forwards compatibility (old library works for new requirements) is rare.

If you have better knowledge, feel free to correct me.

taguan commented 8 years ago

Hi Tuupertunut,

Removing the errors about the private modifiers isn't for future proofing, I get them right now on my project that use the last version of angular2-datatable. Maybe your confusion is that the problem doesn't show up when performing the AOT compilation, but it comes in a later step when compiling the result of the AOT compilation. I read a few threads on the internet, I think it is a know "problem" that won't be corrected, so you need to update your code accordingly.

For the dependencies, I indeed had a problem with peer dependencies. I personally don't like the caret in package.json (something that worked one day is not guaranteed to work the next day). If you have a better proposition to make it work, I would be glad to implement it.

Tuupertunut commented 8 years ago

Ok, let's see. If the first paragraph is true, I should see those errors when trying to AOT compile the included example application. Those errors should then go away when I remove private modifiers from the library itself. Testing...

Tuupertunut commented 8 years ago

Correct. I get the errors when AOT compiling an application that uses angular2-datatable. There are more private modifiers though than what is included in your fix. Namely in DefaultSorter.ts the isSortedByMeAsc/Desc fields and the sort() method.

Edit: I made another pull request #46 with the complete fix.