build(angular): use partial compilation for published angular library

[yharaskrik]

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

• If you've added code that should be tested, please add tests.
• Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

[Lms24]

Hi @yharaskrik, thanks for opening this PR! So I tried setting compilationMode: 'partial' a while ago when changing the Angular SDK's build process (#4641). This was quite some time ago so I might have missed something back then. However, I found that I was getting a build error because setting partial was not yet supported for Angular 10 production builds (i.e. ng build --prod). I'm curious if I missed something back then or if this is still a problem (CI will probably tell us).

Also, remind me if possible, does this actually fix #5268 in the sense that we get rid of the compiler warning? My impression was that ngcc would still be needed to upcompile the SDK to more recent, Ivy-only Angular versions?

[yharaskrik]

Hi @yharaskrik, thanks for opening this PR! So I tried setting compilationMode: 'partial' a while ago when changing the Angular SDK's build process (#4641). This was quite some time ago so I might have missed something back then. However, I found that I was getting a build error because setting partial was not yet supported for Angular 10 production builds (i.e. ng build --prod). I'm curious if I missed something back then or if this is still a problem (CI will probably tell us).

Also, remind me if possible, does this actually fix #5268 in the sense that we get rid of the compiler warning? My impression was that ngcc would still be needed to upcompile the SDK to more recent, Ivy-only Angular versions?

Hmm I ran the builds locally and they worked so it seems the flag is ok? Although I'm unsure if that flag existed in Angular 10. Is there a reason you guys are using Ng 10 to compile your SDK and not a newer version?

Yes it will get rid of that warning. that warning is shown when a library is not compiled with partial or full (ie. Was compiled in the view engine format)

[Lms24]

Is there a reason you guys are using Ng 10 to compile your SDK and not a newer version

Yes, Angular libraries are only forward-compatible, meaning if I compile with ng10, the library will work with all Angular versions starting with ng10 but there's no guarantee for any version below ng10. Our minimum supported Angular version is still Angular 10 which we can't change until we release a new major version. So we can't compile with a newer Angular version. To remove the ngcc warning, we previously decided to drop Angular 10 and 11 in the next major which should enable us to fully support Ivy going forward.

That being said, if your PR works indeed and I missed something when working on #4641 (which could very well be the case), we might just get around this overall. That is, assuming that the partial format continues to work in future Angular versions as well. Do you by any chance happen to have some context around this?

We need to carefully test this change though, meaning we have to build the SDK with this change and test it in every Angular version from 10-15. A good test would include the usage of TraceDirective as it is the one component in our SDK that actually requires Angular-specific compilation.

I might have some time in the next days to test this properly.

[yharaskrik]

I think you are right. I checked with the other GDEs and missed this in the docs. https://angular.io/guide/creating-libraries#publishing-libraries So yes you would need to drop 10 and 11 support. I must not have been building the package correctly somehow or the flag must just be entirely ignored silently since it's unsupported.

…

On Tue., Jan. 3, 2023, 7:26 a.m. Lukas Stracke, ***@***.***> wrote: Is there a reason you guys are using Ng 10 to compile your SDK and not a newer version Yes, Angular libraries are only forward-compatible, meaning if I compile with ng10, the library will work with all Angular versions starting with ng10 but there's no guarantee for any version below ng10. Our minimum supported Angular version is still Angular 10 which we can't change until we release a new major version. So we can't compile with a newer Angular version. To remove the ngcc warning, we previously decided <#5268 (comment)> to drop Angular 10 and 11 in the next major which should enable us to fully support Ivy going forward. That being said, if your PR works indeed and I missed something when working on #4641 <#4641> (which could very well be the case), we might just get around this overall. That is, assuming that the partial format continues to work in future Angular versions as well. Do you by any chance happen to have some context around this? We need to carefully test this change though, meaning we have to build the SDK with this change and test it in every Angular version from 10-15. A good test would include the usage of TraceDirective as it is the one component in our SDK that actually requires Angular-specific compilation. I might have some time in the next days to test this properly. — Reply to this email directly, view it on GitHub <#6636 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACIHZIR3UZFM6K3ERISREJTWQRALPANCNFSM6AAAAAATPBBAGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Lms24]

Hi @yharaskrik so I've tested your changes with Angular 10 and Angular 15. For NG10, things still seem to work (as I expected) but for NG15, I'm getting build errors at application build time, which to me look a lot like Ivy incompatibility errors:

NG13:

ngTest13/node_modules/@angular/core/core"' has no exported member 'ɵɵNgModuleDefWithMeta'.

58     static ɵmod: i0.ɵɵNgModuleDefWithMeta<TraceModule, [typeof TraceDirective], never, [typeof TraceDirective]>;

NG15:

ngTest15/node_modules/@angular/core/index"' has no exported member 'ɵɵFactoryDef'.

36     static ɵfac: i0.ɵɵFactoryDef<SentryErrorHandler, never>;

I'd have loved to have seen this worked as it would have resolved a tricky issue. Unfortunately, there seems no way around dopping Ng10 and 11 support and compiling for Ivy with Ng12.
Please let me know if you have other ideas. Otherwise I'm gonna close this PR.

Thanks for taking a look though, appreciate the help!

[yharaskrik]

Hi @yharaskrik so I've tested your changes with Angular 10 and Angular 15. For NG10, things still seem to work (as I expected) but for NG15, I'm getting build errors at application build time, which to me look a lot like Ivy incompatibility errors:

NG13:

ngTest13/node_modules/@angular/core/core"' has no exported member 'ɵɵNgModuleDefWithMeta'.

58     static ɵmod: i0.ɵɵNgModuleDefWithMeta<TraceModule, [typeof TraceDirective], never, [typeof TraceDirective]>;

NG15:

ngTest15/node_modules/@angular/core/index"' has no exported member 'ɵɵFactoryDef'.

36     static ɵfac: i0.ɵɵFactoryDef<SentryErrorHandler, never>;

I'd have loved to have seen this worked as it would have resolved a tricky issue. Unfortunately, there seems no way around dopping Ng10 and 11 support and compiling for Ivy with Ng12. Please let me know if you have other ideas. Otherwise I'm gonna close this PR.

Thanks for taking a look though, appreciate the help!

Ya, I think you are right. Unfortunately I guess we will have to wait. Thanks anyway!

[Yberion]

Hello @Lms24, just for information, following recent updates from Angular which will probably land with Angular 16, NGCC got removed, meaning that this library won't work anymore with View Engine unless you compile with "compilationMode": "partial".

angular/angular#49101

[Lms24]

Hi @Yberion - we're aware. We recently decided to create a new, ivy compatible package: #5268 (comment)