This repository was archived by the owner on Dec 18, 2024. It is now read-only.
refactor: convert app to latest Angular CLI format #641
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import {MaterialDocsAppPage} from './app.po'; | ||
| import {browser, logging} from 'protractor'; | ||
|
|
||
| describe('Material Docs App', () => { | ||
| let page: MaterialDocsAppPage; | ||
|
|
||
| beforeEach(() => { | ||
| page = new MaterialDocsAppPage(); | ||
| }); | ||
|
|
||
| it('should display welcome message', async() => { | ||
| await page.navigateTo(); | ||
| expect(await page.getTitleText()).toEqual('Angular Material'); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| // Assert that there are no errors emitted from the browser | ||
| const logs = await browser.manage().logs().get(logging.Type.BROWSER); | ||
| expect(logs).not.toContain(jasmine.objectContaining({ | ||
|
There was a problem hiding this comment. I'm not sure whether this will actually fail the test. A long time ago I tried doing something similar to detect leaking elements in unit tests and I couldn't get it to fail properly, although Jasmine could've changed the behavior since then. It might be better to either throw an error here or pull it out into a function that is called at the end of each test. There was a problem hiding this comment. This is taken directly from what the Angular CLI's e2e schematic generates for a new project. If there is a little more concrete guidance here, I could open up an issue against Angular CLI. |
||
| level: logging.Level.SEVERE, | ||
| } as logging.Entry)); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import {browser, by, element} from 'protractor'; | ||
|
|
||
| export class MaterialDocsAppPage { | ||
Splaktar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| navigateTo() { | ||
| return browser.get(browser.baseUrl) as Promise<any>; | ||
| } | ||
|
|
||
| getTitleText() { | ||
| return element(by.css('app-homepage header .docs-header-headline .mat-h1')) | ||
| .getText() as Promise<string>; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,13 @@ | ||
| { | ||
| "extends": "../tsconfig.json", | ||
| "compilerOptions": { | ||
| "outDir": "../out-tsc/e2e", | ||
| "module": "commonjs", | ||
| "target": "es5", | ||
| "types": [ | ||
| "jasmine", | ||
| "jasminewd2", | ||
| "node" | ||
| ] | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,60 +4,65 @@ | |
| "license": "MIT", | ||
| "angular-cli": {}, | ||
| "scripts": { | ||
| "ng": "ng", | ||
| "start": "yarn build-themes && ng serve", | ||
| "start-jit": "yarn build-themes && ng serve --aot=false", | ||
| "start:prod": "yarn build-themes && ng serve --prod", | ||
| "lint": "tslint -p tsconfig.json", | ||
| "test": "yarn build-themes && ng test", | ||
| "pree2e": "webdriver-manager update", | ||
| "e2e": "yarn build-themes && ng e2e", | ||
| "build": "yarn build-themes && ng build", | ||
| "build:highmem": "yarn build-themes && node --max_old_space_size=8192 ./node_modules/@angular/cli/bin/ng build", | ||
|
There was a problem hiding this comment. Is this something commonly required, or is there something special about our app? I'm surprised that it's necessary given the relatively simple nature of our docs app There was a problem hiding this comment. It's tracked here: angular/angular-cli#13734. Based on this issue, it has become quite common and only minor progress has been made towards "solving it". In some cases, the CLI team has stated that increasing the available memory is expected. I only ran into the issue when doing an AOT non-production build. There was a problem hiding this comment. I'm not sure if this would occur with Ivy as well since the Ivy build doesn't get that far into the build process, yet. There was a problem hiding this comment. It's also needed with Ivy. There was a problem hiding this comment. It looks like this is being addressed in PR angular/angular-cli#15777. |
||
| "build:sm": "ng build --prod --source-map", | ||
| "build-themes": "bash ./tools/build-themes.sh", | ||
| "prod-build": "yarn build-themes && ng build --prod", | ||
| "postinstall": "webdriver-manager update --gecko false && yarn build-themes", | ||
| "publish-prod": "bash ./tools/deploy.sh stable prod", | ||
| "publish-dev": "bash ./tools/deploy.sh", | ||
| "publish-beta": "bash ./tools/deploy.sh stable beta" | ||
| }, | ||
| "private": true, | ||
| "dependencies": { | ||
| "@angular/animations": "^8.2.9", | ||
| "@angular/cdk": "^8.2.2", | ||
| "@angular/cdk-experimental": "^8.2.2", | ||
| "@angular/common": "^8.2.9", | ||
| "@angular/compiler": "^8.2.9", | ||
| "@angular/core": "^8.2.9", | ||
| "@angular/forms": "^8.2.9", | ||
| "@angular/material": "^8.2.2", | ||
| "@angular/material-examples": "angular/material2-docs-content", | ||
| "@angular/material-experimental": "^8.2.2", | ||
| "@angular/material-moment-adapter": "^8.2.2", | ||
| "@angular/platform-browser": "^8.2.9", | ||
| "@angular/platform-browser-dynamic": "^8.2.9", | ||
| "@angular/router": "^8.2.9", | ||
| "core-js": "^2.6.9", | ||
| "hammerjs": "^2.0.8", | ||
| "material-components-web": "^4.0.0-alpha.0", | ||
| "moment": "^2.24.0", | ||
| "rxjs": "^6.5.3", | ||
| "zone.js": "^0.9.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@angular-devkit/build-angular": "^0.803.6", | ||
| "@angular/cli": "^8.3.6", | ||
| "@angular/compiler-cli": "^8.2.9", | ||
| "@types/jasmine": "^3.4.1", | ||
| "@types/node": "^12.7.8", | ||
| "firebase-tools": "^7.4.0", | ||
| "jasmine-core": "^3.5.0", | ||
| "jasmine-spec-reporter": "~4.2.1", | ||
| "karma": "^4.3.0", | ||
| "karma-chrome-launcher": "^3.1.0", | ||
| "karma-firefox-launcher": "^1.1.0", | ||
| "karma-jasmine": "^2.0.1", | ||
| "karma-jasmine-html-reporter": "^1.4.2", | ||
| "node-sass": "^4.11.0", | ||
| "protractor": "^5.4.2", | ||
| "ts-node": "^8.4.1", | ||
| "tslint": "^5.20.0", | ||
| "typescript": "3.5.3" | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Are the changes for Angular CLI structure throughout this PR captured by a schematic? To what extent will these changes be required of other Angular CLI users? (Particularly ones who started a project in the early days like us)
cc @vikerman @IgorMinar @StephenFluin
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.
Most of the changes are not breaking or required changes. They mostly fall into the category of best practices. Over time, as these slight deviations from best practices build up, you can run into issues and/or unexpected breaking changes in future migrations.
Most projects that I've seen will try to align themselves with the best practices (i.e. output from a
ng new) from the latest version of the Angular CLI every couple of major releases. Currently, this repo seems to be aligned with the recommendations from Angular CLI v5.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 guess what I really want to know here is how much of this change was for ivy and how much was was just event horizon for Angular CLI?
Uh oh!
There was an error while loading. Please reload this page.
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 just went through and added comments for any Ivy-specific issues (only 2-3).