Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declaration emitter cleanup #4068

Closed
wants to merge 61 commits into from
Closed

Declaration emitter cleanup #4068

wants to merge 61 commits into from

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Jul 29, 2015

This is the first set of changes to get declaration bundling working. the first step is to split the emitter logic into two phases; the first to collect declarations that we need to emit (preprocess) and the second to write them (write). With these changes there should not be any functional changes, just error messages and reporting differences. I will be sending out changes to support --out with --module for declaration and then another set for bundling.

Note for reviewers, declarationEmitter.ts has changed significantly, so i would look at the file as whole instead of diffs. changes to other files are fairly limited though, so diffs should be sufficient.

@@ -1657,7 +1513,7 @@ namespace ts {
// Unnamed function expressions, arrow functions, and unnamed class expressions have reserved names that
// we don't want to display
if (!isReservedMemberName(symbol.name)) {
buildSymbolDisplay(symbol, writer, enclosingDeclaration, SymbolFlags.Type);
buildSymbolDisplay(symbol, writer, enclosingDeclaration, SymbolFlags.Type, undefined, undefined, /*isTypeSymbol*/true);
Copy link
Member

Choose a reason for hiding this comment

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

Use named parameters here.

@DanielRosenwasser
Copy link
Member

In declarationEmitter.ts:

  • Replace all "handel" with "handle"
  • Replace all "infered" with "inferred"
  • Replace all "diffrent" with "different"
  • See where else we use lambdas that do nothing if you find any, let's see if we can move emptyHandler to utilities.ts.
  • Use a for-of in visitExportDeclaration.
  • I'm not sure how consistent action is with the rest of our codebase. I think we've just used visit and callback.
  • Rename reportUnamedDeclarationMessage to reportUnnamedDeclarationMessage.
  • Rename collectDeclatation with collectDeclaration or aggregateDeclaration.
  • In emitDeclarations, don't use diagnostics.push.apply - use addRange.
  • Can you document preprocessDeclarations a little more?

@ahejlsberg ahejlsberg changed the title Declaration emiter cleanup Declaration emitter cleanup Jul 29, 2015
Conflicts:
	src/compiler/declarationEmitter.ts
	src/compiler/types.ts
Conflicts:
	src/compiler/checker.ts
	src/compiler/declarationEmitter.ts
	src/compiler/diagnosticInformationMap.generated.ts
	src/compiler/diagnosticMessages.json
	src/compiler/types.ts
	tests/baselines/reference/chainedImportAlias.symbols
	tests/baselines/reference/es6ImportNamedImportInIndirectExportAssignment.symbols
@mhegazy mhegazy closed this Sep 29, 2015
@mhegazy
Copy link
Contributor Author

mhegazy commented Sep 29, 2015

closing for now. will refresh and resubmit.

@mhegazy mhegazy reopened this Sep 29, 2015
@mhegazy mhegazy closed this Sep 29, 2015
@sheetalkamat sheetalkamat deleted the declarations branch October 10, 2017 18:08
@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.

3 participants