search

valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.53k stars 1.69k forks source link

AoT for datepicker #1093

Closed lifaon74 closed 8 years ago

lifaon74 commented 8 years ago

Re-open : https://github.com/valor-software/ng2-bootstrap/issues/1080

The bug it still there after updating to 1.1.9, please take care to do tests before closing issues, I'll be happy to test it for you if needed. For me it's not an @Hostbinding problem.

Trying to build with ngc (last version installed) : 

import { DatepickerModule } from 'ng2-bootstrap';
@NgModule({
  imports: [DatepickerModule],
})
export class MyModule {}

Results in : 

dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(24,27): error TS2307: Cannot find module './datepicker-inner.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(25,27): error TS2307: Cannot find module './daypicker.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(26,27): error TS2307: Cannot find module './monthpicker.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(27,27): error TS2307: Cannot find module './yearpicker.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(178,18): error TS2341: Property 'datePicker' is private and only accessible within class 'DatePickerComponent'.
dist\tmp\app\node_modules\ng2-bootstrap\components\timepicker\timepicker.component.ngfactory.ts(755,42): error TS2445: Property 'updateHours' is protected and only accessible within class 'TimepickerComponent' and its subclasses.

Seems Datepicker is not AoT ready ;). Hope it will be soon.

Martin-Luft commented 8 years ago

The datePicker property references to the DatePickerInnerComponent. I don't know if you should be allowed to access this property...

The updateHours property I also don't know...

Btw: I don't see the TimepickerModule in your ngModule definition...

@valorkin it's your part :)

lifaon74 commented 8 years ago

Maybe, you could take a look at https://github.com/ocombe/ng2-translate where they created a make.js file to create a proper bundles/ folder with ngc.

My datepicker tag : 

<datepicker
    [(ngModel)]="date"
    (ngModelChange)="onChangeDatePicker()"
    [showWeeks]="false"
    [startingDay]="1"
></datepicker>
valorkin commented 8 years ago

@lifaon74 you don't need bundles

valorkin commented 8 years ago
  1. Cannot find module './datepicker-inner.component.ngfactory'. this is wrong, you should try clean install and build
valorkin commented 8 years ago

https://unpkg.com/ng2-bootstrap/components/datepicker/

valorkin commented 8 years ago

Property 'datePicker' is private and only accessible within class 'DatePickerComponent' and it should be private, if AoT requires to make it public it is wrong

valorkin commented 8 years ago

updateHours is internal method and should not be exposed too

valorkin commented 8 years ago
  1. clean install and build
  2. follow this guide https://github.com/valor-software/ng2-bootstrap/blob/development/docs/getting-started/ng-cli.md
  3. if nothing helped, please reopen and share code sample to reproduce

as for now: internal methods are private as designed

lifaon74 commented 8 years ago

@valorkin Why closing issue if not solved ?

1) Every libs (angular2, ng2-translate, ng2-resource-rest, ng2-component-injector, etc...) have a bundles/ folder with UMD fomat. This is archived by ngc and them use systemjs-builder, not mandatory. It seems you use something different.

2) Of course I clean and build 3)

Property 'datePicker' is private and only accessible within class 'DatePickerComponent' and it should be private, if AoT requires to make it public it is wrong

Why not put it public right now waiting for a bug fix, instead of blocking thousand of users ? same for updateHours

All others libs I use supports AoT, only ng2-boostraps resists ;)

Martin-Luft commented 8 years ago

Is there an official AoT bug issue or should we create one?

valorkin commented 8 years ago

@lifaon74 when I have added system.js bundles there are were no ngc and system.js had problems with UMD format

Can you imagine miss use of internal methods?

lifaon74 commented 8 years ago

ngc comes with @angular/compiler-cli

The bundles are made with "systemjs-builder", I use system-js too and have no problem with ES/UMD, you can have an example here of the config of the systemjs-builder.

valorkin commented 8 years ago

ok, updateHours was used in template, made it public but it is a bit wrong

valorkin commented 8 years ago

@Martin-Wegner there are was an issue about and Rob closed it with as designed

valorkin commented 8 years ago

@lifaon74 it is good reason to make PR

lifaon74 commented 8 years ago

I'll try something this we, but it will probably involve a lot of changes and I dont want to break all of your work.

valorkin commented 8 years ago

Just add in parallel, build to bundles2 for example, than I will try with couple of test projects I have and release

Martin-Luft commented 8 years ago

@Martin-Wegner there are was an issue about and Rob closed it with as designed

Can you post the link to this issue please?

valorkin commented 8 years ago

@Martin-Wegner I am afraid no :(

valorkin commented 8 years ago

it seems any property decorator requires public access modifier not cool

valorkin commented 8 years ago

check v 1.1.10

valorkin commented 8 years ago

@lifaon74 should be working fine now

lifaon74 commented 8 years ago

Hey, good job : 

dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(25,27): error TS2307: Cannot find module './daypicker.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(26,27): error TS2307: Cannot find module './monthpicker.component.ngfactory'.
dist\tmp\app\node_modules\ng2-bootstrap\components\datepicker\datepicker.component.ngfactory.ts(27,27): error TS2307: Cannot find module './yearpicker.component.ngfactory'.

Did you do something about : /datepicker-inner.component.ngfactory ? It disapeared.

valorkin commented 8 years ago

thing is ngfactory never was part of npm module https://unpkg.com/@angular/core@2.0.2/

valorkin commented 8 years ago

@lifaon74 can you join slack and we will speak about it?

Martin-Luft commented 8 years ago

https://ng2.slack.com/messages/ng2-bootstrap/

lifaon74 commented 8 years ago

So, I fork it and fix it here : https://github.com/lifaon74/ng2-bootstrap/commit/da5ff0e3f9c88bbc609cbdd990ed7d1746df7fc3

npm run compile

Build pass for ng2-boostrap and personal projects. I'm currently testing if everything works properly.

Please take a look if I forgot something.

valorkin commented 8 years ago

Do PR and I will play with it

lifaon74 commented 8 years ago

It's not totally ready, I saw you're using a global variable for MouseEvent and Keyboard event and it brokes the execution,, I'll do a PR when it will be ready

lifaon74 commented 8 years ago

So I did a PR but dont have currently enouth time to test everything (CI). It builds and works for one of my project right now. I'll continue to test more later.

lifaon74 commented 8 years ago

Hum, I changed module es2015 to commonjs, for retrocompatibility.

dhardtke commented 8 years ago

I don't know if my issue is related to this one, but I'm also trying to get AOT compilation working. I am using the carousel and I get this:

Error: Error at compiled/app/structure/big-image/big-image.component.ngfactory.ts:20:27: Cannot find module '../../../node_modules/ng2-bootstrap/components/carousel/carousel.component.ngfacto
ry'.
Error at compiled/app/structure/big-image/big-image.component.ngfactory.ts:25:27: Cannot find module '../../../node_modules/ng2-bootstrap/components/carousel/slide.component.ngfactory'.
Error at compiled/app/static/home/home.component.ngfactory.ts:23:27: Cannot find module '../../../node_modules/ng2-bootstrap/components/carousel/carousel.component.ngfactory'.
Error at compiled/app/static/home/home.component.ngfactory.ts:27:27: Cannot find module '../../../node_modules/ng2-bootstrap/components/carousel/slide.component.ngfactory'
dhardtke commented 8 years ago

Okay, got it working by using the commonjs plugin: part of my rollup.js:

commonjs({
    include: ["node_modules/rxjs/**", "node_modules/ng2-bootstrap/**"],
    sourceMap: false
}),

Update: But it isn't working with webpack. Still trying to figure out why.

Update 2: Obviously this has nothing to do with neither webpack nor rollup.

Update 3: Weird. It is working if I place the .ts files from components/carousel in ng2-bootstrap's node_modules/ directory.

dhardtke commented 8 years ago

ping @valorkin Any news on this? The .ngfactory files will only be generated if the .ts files lie around in ng2-bootstrap's folder in node_modules/

valorkin commented 8 years ago

Working on it :(

valorkin commented 8 years ago

I will really appreciate if you will try npm i ng2-bootstrap@beta --save

and try published solution

valorkin commented 8 years ago

BTW I am using ng-cli with --aot flag to test and what are you guys using?

dhardtke commented 8 years ago

I can confirm that it is working. And I'm using webpack with https://github.com/angular/angular-cli/tree/master/packages/webpack

lifaon74 commented 8 years ago

Working for me too, thanks for this great feature !

valorkin commented 8 years ago

just to have your heads up, when I am done with bundling stuff I will publish new datepicker :D

dhardtke commented 8 years ago

Well too early to congratulate: Module not found: Error: Can't resolve '../../../node_modules/ng2-bootstrap/components/carousel/carousel.component.ngfactory' in 'app\static\home'

Could you export the classes from CarouselModule as well?

valorkin commented 8 years ago

@dhardtke try to clean install latest beta version npm i ng2-bootstrap@beta

valorkin commented 8 years ago

because it is exported

Martin-Luft commented 8 years ago

@dhardtke I will close the issue until your feedback is negative.

dhardtke commented 8 years ago

@valorkin:

It works if I clone the repo manually (latest version) and put it in node_modules/.

With npm i ng2-bootstrap@beta it didn't work, same error messages.

dhardtke commented 8 years ago

ping @Martin-Wegner

valorkin commented 8 years ago

Hm, it is strange

dhardtke commented 8 years ago

Okay, I fixed it. First, my npm was behaving weirdly so I had to remove node_modules and install again. But that has nothing to do with why it wasn't working.

Please have a look at this:

// import {CarouselModule} from "ng2-bootstrap/components/carousel/carousel.module";
// import {CollapseModule} from "ng2-bootstrap/components/collapse/collapse.module";
// import {DropdownModule} from "ng2-bootstrap/components/dropdown/dropdown.module";
import {CarouselModule} from "ng2-bootstrap/components/carousel";
import {CollapseModule} from "ng2-bootstrap/components/collapse";
import {DropdownModule} from "ng2-bootstrap/components/dropdown";```

If I was importing the modules like the first, commented statements show, it wouldn't work.
So without "/modulename.module" in the import statements it's working fine with the latest (stable) version.

Thanks again!
valorkin commented 8 years ago

Awesome! Seems I need to simplify file structure so no one will repeat your issue