Skip to content

Use const rather than var when target es6#6597

Merged
DanielRosenwasser merged 6 commits intomicrosoft:masterfrom
plantain-00:use_const_rather_than_var_when_target_es6
Jan 25, 2016
Merged

Use const rather than var when target es6#6597
DanielRosenwasser merged 6 commits intomicrosoft:masterfrom
plantain-00:use_const_rather_than_var_when_target_es6

Conversation

@plantain-00
Copy link
Copy Markdown
Contributor

Comment thread src/compiler/emitter.ts Outdated
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.

<= ScriptTarget.ES5

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.

done

Comment thread src/compiler/emitter.ts Outdated
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.

Can you wrap this in a helper method, since you repeat this pattern 3 times in this PR

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.

done

Comment thread src/compiler/emitter.ts
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.

The function name (emitVar) is misleading and we probably don't need the function at all.

I'd prefer to just have a local in this function const varOrConst = (languageVersion <= ScriptTarget.ES5) ? 'var ' : 'const ';) and write(varOrConst) at these call sites.

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.

done

@RyanCavanaugh
Copy link
Copy Markdown
Member

👍

@paulvanbrenk
Copy link
Copy Markdown
Contributor

👍

Thanks!

DanielRosenwasser added a commit that referenced this pull request Jan 25, 2016
…hen_target_es6

Use const rather than var when target es6
@DanielRosenwasser DanielRosenwasser merged commit 6ff1bbe into microsoft:master Jan 25, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

5 participants