Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

Fix: Label Functions and Methods declartions as Ambient (fixes #162) #163

Closed
wants to merge 1 commit into from

Conversation

soda0289
Copy link
Member

@soda0289 soda0289 commented Feb 19, 2017

Ambient functions do not have a body and will cuase rules to throw
an exception. We use the types TSAmbientFunctionExpression and
TSAmbientMethodDefinition.

@eslintbot
Copy link

LGTM

@soda0289
Copy link
Member Author

soda0289 commented Feb 19, 2017

I noticed in FunctionDeclaration we use the type DeclareFunction when defining ambient function. Should we use TSAmbientFunctionDeclaration as well?

@eslintbot
Copy link

LGTM

@soda0289
Copy link
Member Author

soda0289 commented Feb 20, 2017

I made the change for the node DeclareFunction. It is now TSAmbientFunctionDeclaration to make it align with this pull request. At the very least we should prefix it with TS unless there is another rule that uses DeclareFunction.

…#162)

Ambient functions do not have a body and will cuase rules to throw
an excpetion. We use the types TSAmbientFunctionExpression and
TSAmbientMethoDeclaration.
@eslintbot
Copy link

LGTM

@Pajn
Copy link
Contributor

Pajn commented Mar 23, 2017

When I looked at the test output of this declare class test for #207 I noticed that there is nothing on the class node that make it possible to see that it is declared. That info would be great for prettier so that it can now if it should print declare or not :)
It can of course be added in a different PR, just saw it here.

@soda0289
Copy link
Member Author

@Pajn Yes we do need to label ambient classes. I was thinking of changing the type from Class to TSAmbientClass this way some eslint rules won't crash as they are not aware of empty body methods, which declared classes all have. This would break current linting rules that are meant for classes but it is better then having the crash.

@JamesHenry
Copy link
Member

I'm so sorry it's taken so long to address these PRs @soda0289, so happy to have you on the team now! Let's take stock as soon as we merge in all the breaking changes and release v3!

@soda0289 soda0289 closed this May 6, 2017
@soda0289 soda0289 deleted the fix-ambient-function branch May 23, 2017 13:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants