search

AndrewPoyntz / time-ago-pipe

An Angular pipe for converting a date string into a time ago
MIT License
130 stars 67 forks source link

AoT compilation updates #4

Closed arknotts closed 7 years ago

arknotts commented 7 years ago

Changes needed for AoT compilation. Mostly revolving around using ngc compiler rather than tsc. Also added a module at index.ts or the compiler would complain. This is my first time converting something to use AoT compilation but I followed this guide: https://medium.com/@isaacplmann/getting-your-angular-2-library-ready-for-aot-90d1347bcad#.g1570hrdz

Note: I changed the npm build script from npm run tsc to npm run build since it's no longer using tsc to compile. If you'd rather use something else that's closer to your conventions I'm fine with that, I'll just push another commit.

arknotts commented 7 years ago

Looks like I need to modify the travis config as well, as it tries to run npm run tsc. I'll push another commit tonight.

juergen-vogel commented 7 years ago

Let me know if you need support. There's a second error on the transform function:

Not all code paths return a value.

My AoT compiler complains about that.

arknotts commented 7 years ago

@juergen-vogel Thanks for the offer I'll let you know if I do. That's odd about the transform function, mine compiles fine. What version of @angular/compiler did it pull down? Either way I see how the transform function should be modified to avoid that error and can include it in the commit.

EDIT: Looks like tsc will throw that error if the --noImplicitReturns flag is set. Either way the code can be improved in that spot and I'll include a fix. :slightly_smiling_face:

arknotts commented 7 years ago

@juergen-vogel I changed the last else statement that should fix your compile issue. I tried it with the --noImplicitReturns flag on my machine and it works.

@AndrewPoyntz I updated the travis config file but it still doesn't build on the CI server. Is there a configuration that needs updated to do this? I don't mind changing it back to npm run tsc but I feel it's more descriptive of it's purpose now. EDIT: Nevermind it looks like it passed this time. Not sure why it didn't run build on commit 7881eca!

juergen-vogel commented 7 years ago

@arknotts Thanks for your effort! Works fine! 👍