Skip to content
This repository was archived by the owner on May 22, 2025. It is now read-only.

Conversation

appsforartists
Copy link
Contributor

tsickle's current approach is to run TypeScript, modify the output, and pass it through TypeScript again. This can cause TypeScript to think imports have been declared twice (and fail accordingly).

As @evmar discovered with the internal build of tsickle (b/30708240), this can be worked around by ensuring getDeclarationDiagnostics isn't called on the second pass. A more permanent solution would eliminate the second pass entirely, solving #189.

...program.getOptionsDiagnostics(),
...program.getSyntacticDiagnostics(),
...program.getGlobalDiagnostics(),
];
if (diagnostics.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than this, we should delete this diagnostics check entirely -- we don't care about any errors that are produced here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just this one? Should the other calls to allDianostics.push in toClosureJS be preserved?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just this one, your new change looks right.

@evmar
Copy link
Contributor

evmar commented Oct 5, 2016

Can you change the description to:


tsickle's current approach is to run TypeScript, modify the output, and pass it through TypeScript again. We don't want to type check the code twice, so only check it on the first pass.

Fixes #189.

tsickle's current approach is to run TypeScript, modify the output, and pass it through TypeScript again. We don't want to type check the code twice, so only check it on the first pass.

Fixes #189.
@appsforartists
Copy link
Contributor Author

Done.

@evmar evmar merged commit 2b323fc into angular:master Oct 5, 2016
@evmar
Copy link
Contributor

evmar commented Oct 5, 2016

Thanks!

@appsforartists appsforartists deleted the unpreemit branch October 5, 2016 19:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants