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

include metadatafiles to make aot compatiable. #1004

Closed
wants to merge 1 commit into from

Conversation

ammar91
Copy link

@ammar91 ammar91 commented Oct 3, 2016

I'm able to successfully produce metadata files for primeng by following the below steps.

https://medium.com/@isaacplmann/getting-your-angular-2-library-ready-for-aot-90d1347bcad#.mtna6ixy8

I did make several changes along the way by looking at the error messages during compilation. Those are very minor and quick fixes which are mentioned here (
https://medium.com/@isaacplmann/making-your-angular-2-library-statically-analyzable-for-aot-e1c6f3ebedd5#.nq9gqm4o2), specifically the 3rd one.

However, there were two major changes

  1. Remove typings.json in favor of typescript 2.0, new way of defining type declaration files
    https://blogs.msdn.microsoft.com/typescript/2016/06/15/the-future-of-declaration-files/

  2. Update webpack to webpack2 beta
    This is because uglifyjs plugin break with webpack1, and after upgrading to webpack2, content now being serve from http://localhost:8080 instead of http://localhost:880/webpack-dev-server
    But this should be configurable, just didn't get a chance to look into this one.

  3. Isolate the App Module and Entry file (Application.ts), otherwise webpack complain about Module error described here A dependency to an entry point is not allowed in multiple page app webpack/webpack#2894

Please let me know if anything else needed, I would be happy to assist further.

@ammar91 ammar91 mentioned this pull request Oct 3, 2016
@cagataycivici cagataycivici added the Status: Pending Review Issue or pull request is being reviewed by Core Team label Oct 4, 2016
@cagataycivici cagataycivici added this to the 1.0.0-beta.18 milestone Oct 4, 2016
@adrienverge
Copy link
Contributor

@ammar91 Nice work!

I think you aren't supposed to source the *.metadata.json files in git, it's a lot of noise. They only need to be included in the generated npm package.

Have you managed to produce a build that can be used as a dependency in an Angular project, and successfully compile with ngc? (I couldn't)

@ammar91
Copy link
Author

ammar91 commented Oct 5, 2016

@adrienverge Thanks.

Yes, I'm successfully able to compile my Angular project by having PNG as dependency (with the Pull Request I made).

What errors you are having when you run ngc?

@adrienverge
Copy link
Contributor

When I clone your branch into my project's node_modules/primeng, I get the same error as with regular PrimeNG:

Error: Unexpected value 'CalendarModule' imported by the module 'AppModule'
    at myproject/node_modules/@angular/compiler/bundles/compiler.umd.js:14109:37

(Angular 2.0.1)

@ammar91
Copy link
Author

ammar91 commented Oct 5, 2016

Are you sure you checked out the 'ato' branch instead of master?

@adrienverge
Copy link
Contributor

adrienverge commented Oct 5, 2016

Of course :)

tree node_modules/primeng/components

node_modules/primeng/components
├── accordion
│   ├── accordion.css
│   ├── accordion.d.ts
│   ├── accordion.js
│   ├── accordion.js.map
│   ├── accordion.metadata.json
│   └── accordion.ts
├── autocomplete
│   ├── autocomplete.metadata.json
│   └── ...
├── breadcrumb
│   ├── breadcrumb.metadata.json
│   └── ...
├── button
│   ├── button.metadata.json
│   └── ...
├── calendar
│   ├── calendar.metadata.json
│   └── ...
├── carousel
│   ├── carousel.metadata.json
│   └── ...
...

I just git cloned your branch into node_modules/primeng. Is there any pre-compilation needed? (By the way running npm run ngc in the PrimeNG dir fails with missing Cannot find module 'angular2/core' errors.)

@ammar91
Copy link
Author

ammar91 commented Oct 5, 2016

Does it have primeng.metadata.json next to primeng.d.ts?

node_modules/primeng/primeng.metadata.json

@adrienverge
Copy link
Contributor

Does it have primeng.metadata.json next to primeng.d.ts?

Yes...

By the way, running npm run ngc in the PrimeNG dir fails with Cannot find module 'angular2/core' errors.

@ammar91
Copy link
Author

ammar91 commented Oct 5, 2016

Can you paste your tsconfig.json here?

@adrienverge
Copy link
Contributor

Sure, it's

{
  "compilerOptions": {
    "target": "ES5",
    "module": "es2015",
    "moduleResolution": "node",
    "sourceMap": true,
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "removeComments": true,
    "noImplicitAny": true,
    "noImplicitReturns": true
  },
  "files": [
    "src/app/app.module.ts",
    "src/app/main.ts",
    "typings/browser.d.ts"
  ],
  "exclude": [
    "node_modules",
    "typings/main",
    "typings/main.d.ts"
  ],
  "angularCompilerOptions": {
    "genDir": "aot",
    "skipMetadataEmit" : true
  }
}

Do you have any plunker / demo / open-source project using your ato branch? I could try to start from there.

@ammar91
Copy link
Author

ammar91 commented Oct 5, 2016

@adrienverge
Copy link
Contributor

adrienverge commented Oct 6, 2016

@ammar91 Thanks!!

Unfortunately this proof of concept still doesn't work for me (same error) with tsc 2.0.3 and Angular 2.0.1. I'm opening an issue on your aot-primeng repo so we can continue debugging there: ammar91/aot-primeng#1.

@adrienverge
Copy link
Contributor

Hey, here is some feedback:

  • your aot-primeng prototype now works for me 👍;
  • when I copy its primeng-aot-build/primeng into my project's node_modules/, my project can compile with ngc;
  • when I copy your PrimeNG branch (this pull request) to my project's node_modules/, I get the same error as described previously.

Is there any pre-compilation needed?

@ammar91
Copy link
Author

ammar91 commented Oct 6, 2016

@adrienverge Thanks for your feedback.

Is there any pre-compilation needed?

Yes, that is because when you clone the branch it doesn't contain compiled js files. That is only available in the generated npm package.
So after cloning the branch (this pull request), you should have to run npm run tsc, that would give errors like Cannot find module 'angular2/core but .ts files should be compiled at this point. And after that when you run ngc, there won't be any errors.

@adrienverge
Copy link
Contributor

I'm sorry but this still doesn't work, although *.js and *.metadata.json files are present in node_modules/primeng.

It would be good to have clear instructions on how you proceed to get it working. For instance, the following instructions fail at several points:

git -C /tmp clone https://github.com/ammar91/aot-primeng.git
cd /tmp/aot-primeng
npm install
rm -rf node_modules/primeng
cd node_modules
git clone -b ato https://github.com/ammar91/primeng.git
cd primeng
npm install
npm run tsc
# fails because of missing 'moment' definition files
npm i moment
npm run tsc
# fails because of missing '../compiled/showcase/app.module.ngfactory'
npm run ngc
npm run tsc
cd ../..
ngc -p ./tsconfig.aot.json
# Error: Unexpected value 'DialogModule' imported by the module 'AppModule'

@ammar91
Copy link
Author

ammar91 commented Oct 6, 2016

This works for me without any errors.

git clone https://github.com/ammar91/aot-primeng.git
cd aot-primeng
npm install
rm -rf node_modules/primeng
cd node_modules
git clone -b ato https://github.com/ammar91/primeng.git
cd primeng
npm run tsc (ignore the errors)
cd ../..
npm run ngc

@CelsoSantos
Copy link

What is the status on this? Any help needed?

I'm in dire need of this feature but I'd rather have it when beta18 does come out..

@cagataycivici
Copy link
Member

We'll add AOT support in 1.0.0-RC1 (end of october)

@cagataycivici cagataycivici removed this from the 1.0.0-beta.18 milestone Oct 15, 2016
@ammar91
Copy link
Author

ammar91 commented Oct 15, 2016

Oops, a little disappointed to know this pull request has been closed without merge. Wasn't that helpful?

@cagataycivici
Copy link
Member

I don't think metadata.json files should be under version control, shouldn't we generate them before publishing the npm package instead?

@cagataycivici
Copy link
Member

Let's discuss at 871

@kunshao-msft
Copy link

@cagataycivici curious what the status o 1.0.0-RC1 is, I really need AOT compilation support for my project

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Review Issue or pull request is being reviewed by Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants