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 is compiling #209

Closed
wants to merge 5 commits into from
Closed

AoT is compiling #209

wants to merge 5 commits into from

Conversation

isaacplmann
Copy link
Contributor

I have the library compiling with AoT, but I don't understand how the gulp build process works and the *.metadata.json files are all being generated in the src folder instead of the dist folder. Could you tell me what to change to create those files in the correct place?

So, almost a fix for #206, probably hasn't fixed #207 yet.

@shlomiassaf
Copy link
Owner

@isaacplmann BIG 👍 💯

I appreciate it! I know I will faces lot's of issues soon due to AoT push from google in angular connect.

I'm a little swamped now, I will go over soon and let you know what is needed.

Also, once done can you squash to 1 commit and use angular commit style, this is needed for the changelog.

Thanks!!!

@isaacplmann
Copy link
Contributor Author

isaacplmann commented Sep 29, 2016

I got this working and AoT compiling in my app, but without rollup.

This command makes a local tar.gz that I can install in my app.
npm run build && cp -r src/components/angular2-modal/* dist/ && node_modules/.bin/rimraf dist/**/*[!d].ts dist/*[!d].ts && cd dist && npm pack && cd ..

I'm manually copying the contents of src/components/angular-modal to dist and then deleting all the *.ts files (except the *.d.ts files) from dist.

So this does now fix #207 and #206, even though it breaks rollup.

Note: I also removed /../components from a lot of import paths, which I know I'll need to put back in to make the rollup process work.

@@ -1,7 +1,7 @@
import { NgModule } from '@angular/core';
import { CommonModule } from '@angular/common';

import { ModalModule, Modal as BaseModal } from '../../../../components/angular2-modal';
import { ModalModule, Modal as BaseModal } from '../../../angular2-modal';
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These references used to cause issue with SystemJS and other unpredicted stuff.
I changed them to a full reference and replace them before the build anyway, each is translated to angular2-modal

@shlomiassaf
Copy link
Owner

@isaacplmann Thanks.

I saw the /../components change. There is a process that replaces those to angular-2modal for actual production build since plugins are separate modules.

The rollup outputs UMD, this is important so we will need to sort this out.

There are a lot of files change so inspection will take time.

Can you change back the imports of component/angular2-modal back so I can review it as the final PR?

@isaacplmann
Copy link
Contributor Author

Import paths have been switched back.

@isaacplmann
Copy link
Contributor Author

FYI, I wrote down some of the things I learned making this PR in this article

@@ -25,6 +25,7 @@ export class DialogPreset extends VEXModalContext {
defaultResult: any;
content: Type<any>;
buttons: VEXButtonConfig[];
showInput: any;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it used for?

@@ -43,6 +43,10 @@ export class ModalCustomisationWizard {
fluent.open();
}

public logForm(value: any) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this used for?

@@ -83,6 +83,7 @@ export interface MessageModalPreset extends BSModalContext {
footerClass: string;

buttons: BSMessageModalButtonConfig[];
showInput?: any;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this used for?

@@ -20,6 +20,8 @@ import { BaseDynamicComponent } from './base-dynamic-component';
template: ``
})
export class CSSBackdrop extends BaseDynamicComponent {
public cssClass: string;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of these?

@shlomiassaf
Copy link
Owner

@isaacplmann The TS command you changed is actually not related to the build process but to the UMD bundling process.

It runs after Rollup to make the UMD rollup from bundle an ES5 bundle, that's it.

I will reorder it to run in the right place, this requires programatic invocation of ngc, i'll see if it's possible.

@shlomiassaf
Copy link
Owner

shlomiassaf commented Sep 29, 2016

@isaacplmann ok so now it's more clear, first i'll explain the build/bundle process:

The build process I use is split into 2:

  1. TS to JS
  2. TS to UMD bundle

(1) Has 2 outputs:
- CJS (ES5)
- ES6

Each output is managed by gulp, this allows better control. e.g: adding headers to the files, renaming imports etc...

(2) To create a UMD bundle I have 2 steps:
- Use the output of (1)ES6 to create an ES6 UMD bundle.
- Use TypeScript to transform the ES6 UMD bundle to ES5 UMD bundle.

AoT

There are 2 aspects for AoT compilation support

  1. Support the modular CSJ and ES6 outputs.
  2. Support UDM

I think that (2) does NOT require any action. I think that because I assume metadata.json files are treated as d.ts files by the compiler and as such they are auto imported by the compiler in their original form.
The current d.ts output is provided in the CJS output and automatically used by the compiler/IDE even if UMD bundle is used as main in package.json.

What to do:

So having that in mind we need to programatically run ngc as part of the stream.
This means replacing this code with a ngc gulp plugin that using the compiler-cli programatically.

Once we do that the output directory will contain .metadata.json files

Also, a refactoring is needed, I need to remove the CJS(ES5) build and leave ES6 only since UMD is ES5. This means:

  • Remove CJS scripts
  • Set ES6 TS compilation to output d.ts
  • set main in package.json to refer to UMD bundle

This refactoring is a blocker since ES6 build does not output .d.ts

@shlomiassaf
Copy link
Owner

@isaacplmann I'll take it and let you know if it worked, i'll commit the change to this branch.

@isaacplmann
Copy link
Contributor Author

I assume you figured out why I added public properties on some component classes, but I'll answer explicitly here anyway.

To use AoT, every property that is referenced in a template has to be explicitly declared as public in its component. So all those changes I made were explicitly declaring things that were already in the templates. Also, some private properties I changed to public because the template was using them.

Thanks for working on the build process - I think you'll figure that piece out faster than I could.

@isaacplmann
Copy link
Contributor Author

Something like this might be what you need to use to replace the gulp-typescript plugin.

var exec = require('child_process').exec;

gulp.task('task', function (cb) {
  exec('ngc -p "<path to your tsconfig.json>"', function (err, stdout, stderr) {
    console.log(stdout);
    console.log(stderr);
    cb(err);
  });
});

@shlomiassaf
Copy link
Owner

@isaacplmann that won't help, I need a keep the stream rolling i'll have to programatically wrap ngc with through or something...

@isaacplmann
Copy link
Contributor Author

I came across gulp-exec today. That might do what you need.

On Sep 30, 2016 5:34 PM, "Shlomi Assaf" [email protected] wrote:

@isaacplmann https://github.com/isaacplmann that won't help, I need a
keep the stream rolling i'll have to programatically wrap ngc with through
or something...


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#209 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA0lQIr-LRg615h0J-omt7hYUzkUcDPsks5qvYCBgaJpZM4KJ-jM
.

@shlomiassaf
Copy link
Owner

I'll look into that, I'm not sure it will help though.

You need to understand the process, each file process by TS (or NGC) is not saved into a file rather passed as a memory stream to the next handler in line.
This can be a handler that adds a comment to the file (header), or something that changes the imports ('components/angular2-modal->angular2-modal`) etc...

Going to a shell/childProcess does not allow that, the shell will open a stream and close it, it cant pass the stream...

@shlomiassaf
Copy link
Owner

@isaacplmann ok, got this to work but webpack dev mode now get's angry, will fix that and post.

@shlomiassaf
Copy link
Owner

@isaacplmann well this has been a huge refactor :)

I had to move to webpack 2.0 due to code changes that required better compatibility with TS and rollup.

I also had to remove some build steps, add others and refactor the tsconfig setup.

This project has built in plugins which make things even harder, I need to change the imports from them... this is the ../../../components/angular2-modal import reference from each plugin.
For dev it should stay that way, for packaged prod no.

Now I have to add another step since metadata.json files also has import string literals...

Anyway, big changes, I will need to re-org and see what to do with the PR

@shlomiassaf
Copy link
Owner

Complete PR in #211

@isaacplmann
Copy link
Contributor Author

Closed in favor of #211.

@isaacplmann isaacplmann closed this Oct 3, 2016
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

Successfully merging this pull request may close these issues.

AoT compilation fails using ModalModule.forRoot()
2 participants