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

Proposal: decouple emit/emitDeclarations from the source text #9147

Closed
jasonchoimtt opened this issue Jun 13, 2016 · 8 comments
Closed

Proposal: decouple emit/emitDeclarations from the source text #9147

jasonchoimtt opened this issue Jun 13, 2016 · 8 comments
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@jasonchoimtt
Copy link

TypeScript Version:

nightly (1.9.0-dev.20160217)

Use case:

We are improving the coverage of ts-api-guardian which scans a public api surface exposed via a single entry point and serializes it for later comparison. The idea to improve the coverage is to serializing the public api surface into text output compatible with .d.ts file format.

For example, given

// a.ts
export class A { p: number; }

// index.ts
export * from './a';

we want to create a file with

// index.d.ts
export class A { p: number; }

To do this, we'd like to reuse as much of the declaration emitter from typescript as possible so that we are in sync as new access modifiers, built-in types and symbol flags are added to typescript.

One approach we tried was to create collect all exported symbols, create corresponding synthesized nodes for them and then emit the declaration file for this AST, thus reusing existing emitDeclarations function. However this function is tightly coupled to the underlying SourceFile and uses the source text to output identifiers and types. As a result, calling emit on a synthesized SourceFile (without source text) gives something like:

export class  { : ; }

Would it be possible to either have

  1. Typescript exposes functions to emit a declaration string for for individual module exports; or
  2. emitDeclarations work with synthesized nodes (without source text).
    Proposal: Replace emitter with syntax tree transformations and a simplified node emitter. #5595 seems related (we could transform the AST to inline all the reexports), but does not account for the declaration emitter.
@mhegazy
Copy link
Contributor

mhegazy commented Jun 13, 2016

We do need both the SourceFile and an EmitResolver to be able to emit declarations. we use the source text for comments (as comments are not stored on the tree, but fished out from the source text when needed).

in theory, with #5595 in place, the declaration emitter can be rewritten as a transformation (e.g. remove function bodies, annotate unannotated variables, add a declare modifier, etc..). If that is done, it should work on synthesized nodes, with no parent pointers, or source text; but this is not something that will happen in the short term or medium term.

I would have expected that you would compare 2.d.ts files instead of looking at the sources. can you elaborate on your approach?

@jasonchoimtt
Copy link
Author

My example might have been confusing, but we do intend to generate the serialized format from the d.ts files. However, that still requires us to resolve all re-exports and export aliases to create a single d.ts file with all symbols, so we still need to manipulate the AST nodes.

The picture is that we will check in a serialized form of the public api surface into version control. A test step then ensures that a given commit does not change the public api surface, by generating the serialized format for the new code and asserting that it is the same.

The d.ts format presents itself naturally as a serialized format of the api surface. I think it is evolutionarily important for typescript to eventually work with synthesized nodes too.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 14, 2016

but we do intend to generate the serialized format from the d.ts files

so why is not the .d.ts enough to do the comparison.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 14, 2016

let me elaborate. i would think you have two .d.ts files, new.d.ts and old.d.ts, then you would load both, walk the declarations in the old, and make sure they are not removed, and for modified ones, make sure they are "equivalent".

not sure i see the need for yet another format to do this.

@jasonchoimtt
Copy link
Author

That's the idea, if there were only one single d.ts file for the module. This is not the case for us, so we have to create the new/old.d.ts files by ourselves. Imagine the module export is something like

// index.d.ts
export * from './utils';
export { A, B, C } from './core';

// utils.d.ts
export { D, E } from './foo';
export * from './bar';

// etc.

then we have to walk multiple files to generate a "flattened" d.ts file. This is the "format" we need, and is where we hope to reuse the parser and declaration emitter.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 14, 2016

Then i think you would benefit more from #4433

@IgorMinar
Copy link

yeah #4433 would help us. In the meantime I think we'll just generate a single dts file via outFile option, parse it, find all the exported declarations and extract the source text for these declarations that we'll then output as a new file. We'll be missing imports of external symbols but we don't really care about those since we are not trying to construct a valid d.ts file.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@weswigham
Copy link
Member

Our printer supports synthesized nodes and our declaration emitter is now a (typechecker-dependent) tree transformation, and API users can register custom after transformers for declarations, so I'm pretty sure the original ask of this issue is in fact complete nowadays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

5 participants