Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

More factory functions#15531

Merged
rbuckton merged 2 commits into
mastermicrosoft/TypeScript:masterfrom
moreFactoryFuncsmicrosoft/TypeScript:moreFactoryFuncsCopy head branch name to clipboard
May 8, 2017
Merged

More factory functions#15531
rbuckton merged 2 commits into
mastermicrosoft/TypeScript:masterfrom
moreFactoryFuncsmicrosoft/TypeScript:moreFactoryFuncsCopy head branch name to clipboard

Conversation

@rbuckton

@rbuckton rbuckton commented May 2, 2017

Copy link
Copy Markdown
Contributor

This adds a number of missing factory functions for anyone writing custom transformers, as well as performs some cleanup in factory.ts.

Note, there are a few breaking API name changes in this PR as part of the public factory functions. I feel this is acceptable as custom transforms are still a fairly experimental feature, though if necessary we could export aliases for the renamed functions for backwards compatibility.

There's also a small change to the transform unit tests so that they aren't dependent on the new-line format of the host platform running the tests.

This builds on #15500 and #15511.

Comment thread src/compiler/factory.ts

export function createKeywordTypeNode(kind: KeywordTypeNode["kind"]) {
return <KeywordTypeNode>createSynthesizedNode(kind);
export function createMethod(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, asteriskToken: AsteriskToken | undefined, name: string | PropertyName, questionToken: QuestionToken | undefined, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined, body: Block | undefined) {

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.

why make the Declaration part of the name implicit?

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.

Consistency with most other factory functions. Most of the time we're transforming to JavaScript, and JavaScript doesn't have method signatures.

Comment thread src/compiler/factory.ts
export function createParenthesizedType(type: TypeNode) {
const node = <ParenthesizedTypeNode>createSynthesizedNode(SyntaxKind.ParenthesizedType);
/* @internal */
export function createSignatureDeclaration(kind: SyntaxKind, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined) {

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.

In analogue to the above, should this be createSignature?

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.

Per above, signatures are purely TypeScript. We can probably change the name if only to make it more concise.

Comment thread src/compiler/factory.ts
// Type Elements
// Signature elements

export function createSignatureDeclaration(kind: SyntaxKind, typeParameters: TypeParameterDeclaration[] | undefined, parameters: ParameterDeclaration[], type: TypeNode | undefined) {

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.

I can't really tell from the diff if the code was merely re-ordered or if there are more substantive changes. Looks similar on the LHS and RHS though.

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.

Mostly reordered to align with the order in SyntaxKind. It makes finding things easier.

@rbuckton rbuckton merged commit 47bd4d3 into master May 8, 2017
@rbuckton rbuckton deleted the moreFactoryFuncs branch May 8, 2017 19:05
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.