build: enable bundle_dts for upgrade package

[alan-agius4]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• angular.io application / infrastructure changes
• Other... Please describe:

What is the current behavior?

At the moment, the API extractor doesn't support local namespaced imports, this will break the generation of flat dts files. When we turn on dts bundling for this package it will break. Hence this is the ground work needed for making this package compatable with the API extractor.

Relates to #28588

[petebacondarwin]

Given that ngtsc will not work with api-extractor, why are we not putting our energy into fixing that rather than working around it for this package?

[alan-agius4]

Good point, but since we potential want to roll this out for version 8, for the time being the dts bundler will be enabled only under the legacy ngc compiler.

While indeed ngtsc does generate namespaced imports, it could generate named imports if absolutely needed. So eventually after version 8 is release we have 2 options, if we see that this feature has not been implemented by Microsoft.

1. Pitch in and help to implement this feature, which is not a small one.
2. Change ngtsc to emit named imports.

//cc @alexeagle, @IgorMinar & @alxhub as there are also aware of this issue.

[alexeagle]

Another possibility is to keep generating individual .d.ts for this one package.

[alan-agius4]

Yeah, that’s another option. How’s gonna make the call for that?

Is: merge this and enable flattening dts or keep this package as is?

[alxhub]

I'm very wary of us ending up in a state where only a subset of TS will be supported within the Angular repo and as a compilation target. I don't see the "don't use namespaced imports" solution as being viable long-term.

[alexeagle]

@alxhub I agree somewhat, but also, we've always had to stay within some subset (eg. worked with ts2dart, now has to work with closure compiler)
We know that long-term, we need to make fixes in the dts-bundler so the question is how we want these packages to look for v8. APFv8 will say that bundling .d.ts is optional, so I think either way is valid.

[IgorMinar]

can you please reference microsoft/rushstack#1029 from the commit that removes the namespaces.

also will the build actually break with an (semi) intelligent error when someone reintroduces namespaces or will it just produce a broken artifact? if it's the former then I think we can go ahead with this change otherwise we should discuss how to prevent namespace reintroduction.

[IgorMinar]

do we have an estimate for fixing microsoft/rushstack#1029? is that something we could do ourselves? or is it already on the short-term roadmap of that team?

[alan-agius4]

@IgorMinar, the commit msg has been updated to reference the issue mentioned above.

API extractor, will emit a clear error whenever a local namespaced import is found. Something along "import * as __ from __" for local paths is not supported.

Regarding the ETA/roadmap for this issue to be solved, I reached out to Pete G, from Microsoft. When, I have an answer I'll let you know. To make it clear, this is not a bug but rather a new feature request.

[alan-agius4]

CI is flaky, can someone re-triggering it please? Thanks

[IgorMinar]

merge-assistance: check http://test/OCL:235139957:BASE:235139984:1550820856416:dc41396
and merge plz

[alan-agius4]

@IgorMinar, Pete from Microsoft replied.

@alan-agius4 it would be awesome if someone in the community wanted to help out with microsoft/rushstack#1029 to get it implemented sooner.

However it may be somewhat challenging for a first PR. I haven't thought about the design enough yet to know how much work it is. But I will point out that this construct allows arbitrary aliasing of types, e.g. you can say export { A } from './file'; and also import * as N1 from './file'; export { N1 } and now your entry point has A and N1.A and maybe N2.A in its exports. What is the "main" name for A? There isn't a natural choice. This issue doesn't generally arise with the namespace keyword.

On the other hand, if we look at the various "real life" examples people submitted, maybe we can take a shortcut and implement something simple that works for the most common cases. Again, if people want to do some investigation or post some suggestions, that might help push this along a little faster.

[IgorMinar]

thanks @alan-agius4 for the followup. I'll merge this for now.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}