Skip to content

Minor cleanup in bindNamespaceExportDeclaration - #27367

Merged
2 commits merged into
masterfrom
bindNamespaceExportDeclaration
Oct 1, 2018
Merged

2 commits merged into
masterfrom
bindNamespaceExportDeclaration

Conversation

@ghost

Copy link
Copy Markdown

Broken out from #27281

  • There's a mix of if-else and also early returns. This makes the three cases look different, but they're actually all the same -- file a diagnostic and return.
  • They also all add the diagnostic in the same way and are distinguished only by the name. So we can put the diagnostic-adding code in one place.

Comment thread src/compiler/binder.ts Outdated
file.bindDiagnostics.push(createDiagnosticForNode(node, Diagnostics.Global_module_exports_may_only_appear_at_top_level));
return;
const diag = !isSourceFile(node.parent)
? Diagnostics.Global_module_exports_may_only_appear_at_top_level

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.

Since this is a cleanup, can you instead use the syntax

preducate1 ? result1 :
predicate2 ? result2 :
undefined

I think that's a lot easier to read.

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.

Formatting change request

@ghost

Copy link
Copy Markdown
Author

Nathan Shively-Sanders (@sandersn) good to merge?

@ghost
ghost merged commit 115636b into master Oct 1, 2018
@ghost
ghost deleted the bindNamespaceExportDeclaration branch October 1, 2018 19:15
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
This pull request was closed.
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.

1 participant