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 for datepicker #1093

Closed
lifaon74 opened this issue Oct 6, 2016 · 49 comments
Closed

AoT for datepicker #1093

lifaon74 opened this issue Oct 6, 2016 · 49 comments

Comments

@lifaon74
Copy link

lifaon74 commented Oct 6, 2016

Re-open : #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
Copy link
Contributor

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
Copy link
Author

lifaon74 commented Oct 6, 2016

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
Copy link
Member

valorkin commented Oct 7, 2016

@lifaon74 you don't need bundles

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

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

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

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
Copy link
Member

valorkin commented Oct 7, 2016

updateHours is internal method and should not be exposed too

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

  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

@valorkin valorkin closed this as completed Oct 7, 2016
@lifaon74
Copy link
Author

lifaon74 commented Oct 7, 2016

@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

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
Copy link
Contributor

Martin-Luft commented Oct 7, 2016

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

@Martin-Luft Martin-Luft reopened this Oct 7, 2016
@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

@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
Copy link
Author

lifaon74 commented Oct 7, 2016

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
Copy link
Member

valorkin commented Oct 7, 2016

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

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

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

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

@lifaon74 it is good reason to make PR

@lifaon74
Copy link
Author

lifaon74 commented Oct 7, 2016

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
Copy link
Member

valorkin commented Oct 7, 2016

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

@Martin-Luft
Copy link
Contributor

Martin-Luft commented Oct 7, 2016

@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
Copy link
Member

valorkin commented Oct 7, 2016

@Martin-Wegner I am afraid no :(

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

it seems any property decorator requires public access modifier
not cool

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

check v 1.1.10

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

@lifaon74 should be working fine now

@lifaon74
Copy link
Author

lifaon74 commented Oct 7, 2016

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 valorkin reopened this Oct 7, 2016
@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

thing is ngfactory never was part of npm module
https://unpkg.com/@angular/[email protected]/

@valorkin
Copy link
Member

valorkin commented Oct 7, 2016

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

@Martin-Luft
Copy link
Contributor

@lifaon74
Copy link
Author

lifaon74 commented Oct 7, 2016

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
Copy link
Member

valorkin commented Oct 7, 2016

Do PR and I will play with it

@lifaon74
Copy link
Author

lifaon74 commented Oct 7, 2016

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
Copy link
Author

lifaon74 commented Oct 7, 2016

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
Copy link
Author

lifaon74 commented Oct 7, 2016

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

@dhardtke
Copy link

dhardtke commented Oct 9, 2016

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
Copy link

dhardtke commented Oct 9, 2016

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
Copy link

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
Copy link
Member

Working on it :(

@valorkin
Copy link
Member

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

and try published solution

@valorkin
Copy link
Member

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

@dhardtke
Copy link

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

@lifaon74
Copy link
Author

Working for me too, thanks for this great feature !

@valorkin
Copy link
Member

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

@dhardtke
Copy link

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?

@Martin-Luft Martin-Luft reopened this Oct 12, 2016
@valorkin
Copy link
Member

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

@valorkin
Copy link
Member

because it is exported

@Martin-Luft
Copy link
Contributor

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

@dhardtke
Copy link

dhardtke commented Oct 12, 2016

@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
Copy link

ping @Martin-Wegner

@valorkin
Copy link
Member

Hm, it is strange

@dhardtke
Copy link

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
Copy link
Member

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

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

No branches or pull requests

4 participants