-
Notifications
You must be signed in to change notification settings - Fork 174
refactor(app-shell): redo project layout, build system, test runner, package layout, and exports #102
Conversation
jeffbcross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a couple of nits, questions and changes.
app-shell/gulpfile.js
Outdated
| }); | ||
|
|
||
| // Trampoline into gulpfile.ts. | ||
| require('./gulpfile.ts'); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recently learned that with ts-node, you don't need this file anymore. You'll have to delete declare var require from gulpfile.ts, but then you can just run gulp build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
| "tslint": "^3.6.0", | ||
| "typescript": "^2.0.2", | ||
| "typings": "^1.0.4" | ||
| "@angular/common": "^2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Angular, Zone and RxJS dependencies should be peer dependencies in the published package.json. Fine to move them to dev dependencies here though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @angular/core is the only real peer dep.
app-shell/src/app/index.ts
Outdated
|
|
||
| export * from './module'; | ||
| export * from './prerender'; | ||
| export * from './shell'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
app-shell/src/app/module.ts
Outdated
| providers: [{provide: IS_PRERENDER, useValue: false}], | ||
| }; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
app-shell/src/app/prerender.ts
Outdated
| @@ -0,0 +1,3 @@ | |||
| import {OpaqueToken} from '@angular/core'; | |||
|
|
|||
| export const IS_PRERENDER = new OpaqueToken('IsPrerender'); No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
app-shell/src/index.ts
Outdated
| @@ -0,0 +1 @@ | |||
| export * from './app'; No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
app-shell/tsconfig.es5.json
Outdated
| "src/unit_tests.ts" | ||
| ] | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
app-shell/tsconfig.esm.json
Outdated
| "rootDir": ".", | ||
| "baseUrl": ".", | ||
| "paths": { | ||
| "@angular/service-worker/worker": ["src/worker"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy-paste error - gone.
app-shell/tsconfig.json
Outdated
| "exclude": [ | ||
| "node_modules" | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline plz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
7181131 to
5867cad
Compare
…package layout, and exports Also moves shell-parser to an experimental directory until it can be updated.
5867cad to
ad36aa9
Compare
Also moves shell-parser to an experimental directory until it can be updated.