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

chore(Conventions): naming standards #196

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"tslint-loader": "^2.1.0",
"typedoc": "^0.3.12",
"typescript": "^1.7.3",
"typings": "^0.3.1",
"typings": "^0.4.1",
Copy link
Owner

Choose a reason for hiding this comment

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

can you make a separate pull-request for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

"url-loader": "^0.5.6",
"webpack": "^1.12.9",
"webpack-dev-server": "^1.12.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import {
} from 'angular2/testing';

// Load the implementations that should be tested
import {App} from './app';
import {AppCmp} from './app.component';

describe('App', () => {
// provide our implementations or mocks to the dependency injector
beforeEachProviders(() => [
App
AppCmp
]);

it('should have a url', inject([ App ], (app) => {
it('should have a url', inject([ AppCmp ], (app) => {
expect(app.url).toEqual('https://twitter.com/AngularClass');
}));

Expand Down
8 changes: 4 additions & 4 deletions src/app/app.ts → src/app/components/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {Component} from 'angular2/core';
import {RouteConfig, Router, ROUTER_DIRECTIVES} from 'angular2/router';
import {FORM_PROVIDERS} from 'angular2/common';

import {Home} from './home/home';
import {HomeCmp} from './home/home.component';

/*
* App Component
Expand Down Expand Up @@ -36,10 +36,10 @@ import {Home} from './home/home';
`
})
@RouteConfig([
{ path: '/', component: Home, name: 'Index' },
{ path: '/home', component: Home, name: 'Home' }
{ path: '/', component: HomeCmp, name: 'Index' },
{ path: '/home', component: HomeCmp, name: 'Home' }
])
export class App {
export class AppCmp {
Copy link
Owner

Choose a reason for hiding this comment

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

if it's .component then the name should also be AppComponent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly is the going standard out there.
I guess this PR is almost more for discussion :)

I just shutter at the thought of naming all components with full suffix Component.... just for simplicity sake. When shorthand is ample, why not I guess.

Copy link
Owner

Choose a reason for hiding this comment

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

if Cmp then .cmp.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

^ I like that. At end of day, I like to use what is universally found in the community. It just makes bringing new people into a project easier because they see what they are used to and have seen everywhere else.

So I like that personally, but don't want to use it if my project would be the only one using it ;)

name = 'Angular 2 Webpack Starter';
url = 'https://twitter.com/AngularClass';
constructor() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import {BaseRequestOptions, Http} from 'angular2/http';
import {MockBackend} from 'angular2/http/testing';

// Load the implementations that should be tested
import {Home} from './home';
import {Title} from './providers/title';
import {HomeCmp} from './home.component';
import {Title} from '../../providers/title.provider';

describe('Home', () => {
// provide our implementations or mocks to the dependency injector
beforeEachProviders(() => [
Title,
Home,
HomeCmp,
BaseRequestOptions,
MockBackend,
provide(Http, {
Expand All @@ -29,15 +29,15 @@ describe('Home', () => {
deps: [MockBackend, BaseRequestOptions]})
]);

it('should have a title', inject([ Home ], (home) => {
it('should have a title', inject([ HomeCmp ], (home) => {
expect(home.title.value).toEqual('Angular 2');
}));

it('should have a http', inject([ Home ], (home) => {
it('should have a http', inject([ HomeCmp ], (home) => {
expect(!!home.http).toEqual(true);
}));

it('should log ngOnInit', inject([ Home ], (home) => {
it('should log ngOnInit', inject([ HomeCmp ], (home) => {
spyOn(console, 'log');
expect(console.log).not.toHaveBeenCalled();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {Component} from 'angular2/core';
import {Component, OnInit} from 'angular2/core';
import {FORM_DIRECTIVES} from 'angular2/common';
import {Http} from 'angular2/http';

import {Title} from './providers/title';
import {XLarge} from './directives/x-large';
import {Title} from '../../providers/title.provider';
import {XLarge} from '../../directives/x-large.directive';

@Component({
// The selector is what angular internally uses
// for `document.querySelectorAll(selector)` in our index.html
// for document.querySelectorAll(selector) in our index.html
// where, in this case, selector is the string 'app'
selector: 'home', // <home></home>
// We need to tell Angular's Dependency Injection which providers are in our app.
Expand All @@ -27,7 +27,7 @@ import {XLarge} from './directives/x-large';
// Every Angular template is first compiled by the browser before Angular runs it's compiler
template: require('./home.html')
})
export class Home {
export class HomeCmp implements OnInit {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though using ngOnInit will work without implementing OnInit, it appears to be recommended to still implement for TypeScript sake:
angular/angular#5814

Copy link
Owner

Choose a reason for hiding this comment

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

I didn't include this on purpose

// TypeScript public modifiers
constructor(public title: Title, public http: Http) {

Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {BaseRequestOptions, Http} from 'angular2/http';
import {MockBackend} from 'angular2/http/testing';

// Load the implementations that should be tested
import {XLarge} from './x-large';
import {XLarge} from './x-large.directive';

describe('x-large directive', () => {
// Create a test component to test directives
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
beforeEachProviders,
TestComponentBuilder
} from 'angular2/testing';
import {Title} from './title';
import {Title} from './title.provider';

describe('Title', () => {
let title;
Expand Down
File renamed without changes.
4 changes: 2 additions & 2 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import {HTTP_PROVIDERS} from 'angular2/http';
* App Component
* our top level component that holds all of our components
*/
import {App} from './app/app';
import {AppCmp} from './app/components/app.component';
/*
* Bootstrap our Angular app with a top level component `App` and inject
* our Services and Providers into Angular's dependency injection
*/
document.addEventListener('DOMContentLoaded', function main() {
bootstrap(App, [
bootstrap(AppCmp, [
...('production' === process.env.ENV ? [] : ELEMENT_PROBE_PROVIDERS),
...HTTP_PROVIDERS,
...ROUTER_PROVIDERS,
Expand Down
25 changes: 15 additions & 10 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@
"sourceMap": true
},
"files": [
"typings/main.d.ts",
"test/injector.spec.ts",
"test/sanity-test.spec.ts",
"src/app/app.spec.ts",
"src/app/home/home.spec.ts",
"src/app/home/directives/x-large.spec.ts",
"src/app/home/providers/title.spec.ts",
"src/main.ts",
"src/vendor.ts"
"./src/app/components/app.component.spec.ts",
"./src/app/components/app.component.ts",
"./src/app/components/home/home.component.spec.ts",
"./src/app/components/home/home.component.ts",
"./src/app/directives/x-large.directive.spec.ts",
"./src/app/directives/x-large.directive.ts",
"./src/app/providers/title.provider.spec.ts",
"./src/app/providers/title.provider.ts",
"./src/main.ts",
"./src/vendor.ts",
"./test/injector.spec.ts",
"./test/sanity-test.spec.ts"
],
"filesGlob": [
"./src/**/*.ts",
Expand All @@ -26,5 +29,7 @@
],
"compileOnSave": false,
"buildOnSave": false,
"atom": { "rewriteTsconfig": true }
"atom": {
"rewriteTsconfig": true
}
}