Skip to content

treat ambient non-aliased 'require' as commonjs 'require' - #11819

Merged
Vladimir Matveev (vladima) merged 1 commit into
masterfrom
vladima/11752
Oct 24, 2016
Merged

treat ambient non-aliased 'require' as commonjs 'require'#11819
Vladimir Matveev (vladima) merged 1 commit into
masterfrom
vladima/11752

Conversation

@vladima

Copy link
Copy Markdown
Contributor

fixes #11752

Comment thread src/compiler/checker.ts
return getReturnTypeOfSignature(signature);
}

function isCommonJsRequire(node: Node) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May be we need to use this function in completions.ts to determine whether to give module names as completion list?

Comment thread src/compiler/checker.ts
return getReturnTypeOfSignature(signature);
}

function isCommonJsRequire(node: Node) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May be we need to use this function in completions.ts to determine whether to give module names as completion list?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Leave a comment

Comment thread src/compiler/checker.ts
return getReturnTypeOfSignature(signature);
}

function isCommonJsRequire(node: Node) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

May be we need to use this function in completions.ts to determine whether to give module names as completion list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

certainly but it is not related to the original issue so I'd put it into different PR

@mhegazy Mohamed Hegazy (mhegazy) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should make sure completion is also behaving the same way.

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.

commonjs module functionality is broken when project externally defined ambient require function

4 participants