Skip to content

Changed TypeScript index.module classes to functions #635

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

Merged
merged 3 commits into from
Jun 26, 2015

Conversation

mikesigs
Copy link
Contributor

This fixes #629.

index.ts was split into several files: index.module.ts, index.config.ts, index.route.ts, and index.run.ts. The last three files in that list each defined classes to encapsulate their functionality. However, in the index.module.ts, the classes were being passed to the angular module.config() and module.run(), but Angular doesn't call these functions with the new keyword, meaning they should just be functions. Having them as classes didn't actually break anything, but when trying to use this in the constructor, you got undefined (in strict mode) or window otherwise. Changing these implementations to just export function will prevent devs from writing code as though it's a constructor.

@zckrs
Copy link
Collaborator

zckrs commented Jun 25, 2015

lgtm 👍

@zckrs
Copy link
Collaborator

zckrs commented Jun 25, 2015

@mikesigs Do you have already written unit test in TypeScript with Karma ? I look for help or a good example.

@Swiip
Copy link
Owner

Swiip commented Jun 25, 2015

Just a question, if we change the type from class to function, shouldn't we change the first character of the name of a lower case?

@zckrs
Copy link
Collaborator

zckrs commented Jun 25, 2015

Agree

@mikesigs
Copy link
Contributor Author

I lower-cased the function names.

@zckrs No, I'm afraid that's not something I've tackled yet. But I agree, there aren't too many great examples out there.

@zckrs
Copy link
Collaborator

zckrs commented Jun 25, 2015

@mikesigs
Copy link
Contributor Author

Thanks zckrs. It's fixed now. Really wish I could get these tests to work locally. Sorry about that.

@zckrs
Copy link
Collaborator

zckrs commented Jun 26, 2015

No problem :-) Travis ensure.
Thank you for contributions.

zckrs pushed a commit that referenced this pull request Jun 26, 2015
Changed TypeScript index.module classes to functions
@zckrs zckrs merged commit ccf28f2 into Swiip:master Jun 26, 2015
@pgrm
Copy link

pgrm commented Jun 26, 2015

👍

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.

this is undefined in RouterConfig class in TypeScript
4 participants