fix(angular): Use Angular compiler to compile @sentry/angular

[Lms24]

Background

As brought to attention in #3282, our Angular SDK currently causes a compiler error when including TraceDirective (or TraceModule) in an Angular application.

Problem

The root cause for this problem is that the Angular SDK (as most our SDKs) is currently compiled with tsc. While this works fine for regular library interaction, it is a problem for Angular specific SDK items, such as TraceDirective or TraceModule. The reason is that tsc treats these classes (and their decorators) like a normal typescript file and compiles them with the experimental decorators to JS. Angular and the Angular Runtime however, need a specific format which tsc does not deliver. A more detailed explanation on the compiler differences is given in #4644.

Upon application compilation, the Angular compiler notices that TraceDirective is not compiled correctly and thus raises the compiler error mentioned in the issue.

Solution

The solution is to switch from tsc to the Angular compiler to compile our Angular SDK. This is done via the Angular CLI which internally uses ngc and ng-packagr to build and package our SDK correctly so that the Angular runtime can process all SDK components (including TraceDirective) correctly.

Important: This is a breaking change as it requires SDK users to use Angular >= 10. Currently, our SDK might support earlier Angular versions despite the fact that we only list Angular 10-13 as peer dependencies of the SDK. With this new compiler, applications using the Sentry Angular SDK on earlier versions than Angular 10 will not be able to compile correctly.

Angular compiler and APF

The angular compiler is added to this project via several devDependencies, including the Angular CLI which serves as a top level wrapper around ngc and ng-packagr. We use the CLI in our NPM scripts to build the library. The resulting build structure conforms to the Angular Package Format (AFP) v10. Basically, the NPM script build triggers the ng build --prod command which creates a production build adhering to the APF. As a consequence, the package format of this SDK now differs significantly from our other SDKs (or from the current @sentry/angular SDK:

build/
├─ bundles/
│  ├─ UMD JS files (+maps, +min)
├─ esm2015/
│  ├─ ES2015 JS files
├─ fesm2015/
│  ├─ Flattened ES2015 JS files
├─ README.md
├─ *d.ts files
├─ package.json
├─ ...

• UMD to use it in node or for bundlers that don't work w/ (F)ESM
• ESM2015 as a fallback for bundlers that don't work with FESM
• FESM2015 as a default for the Angular CLI and for bundlers

The main point in which the APF differs from our current packaging procedure:
Everything that should go into the NPM tarball is first compiled by ngc and then packaged and copied to build by ng-packagr. This means, we do not use prepack.ts to make changes to the central build directory like in most other SDKs.

Building

This PR changes some yarn scripts by making the Angular compiler the default builder and providing a "legacy" option to build and pack a tsc-compiled package.

Note: These NPM scripts are a proposal and their applicability remains TBD at the moment!

• yarn build builds APF conform JS with the Angular compiler to ./build.
• yarn build:legacy builds CJS and ESM modules with tsc to ./dist and ./esm respectively
• Watch mode works analogously for both compilers the angular compiler by adding :watch to the command
• yarn build:npm creates a tgz archive from the APF build in .
• yarn build:npm:legacy creates a tgz from the tsc build in .

EDIT: We decided to hold off from creating a legacy package for the moment.

A note on package.json

When running yarn build, everything is built with ngc and packaged with ng-packagr into one directory, namely ./build. During packaging, ng-packagr will take the original package.json, make some changes to it and write the new version to ./build/package.json. The made changes are related to the entry points and they differ as follows:

• main and module no longer point to esm and dist resp. but to bundles
• typings is newly introduced and points to the generated d.ts entry file (sentry-angular.d.ts)
• es2015 and fesm2015 are newly introduced and point to ./fesm2015
• esm2015 is newly introduced and points to ./esm2015
• metadata is newly introduced and points to sentry-angular.metadata.json which contains some Angular AOT compilation metadata

Testing

As of this moment, there are no unit or integration tests available for @sentry/angular.
What I primarily did to test this was to create four Angular projects (versions 10-13) and install the tgz NPM package locally to these projects.

These very basic applications can 1) throw sample errors, 2) create custom spans and 3) test TraceDirectve to ensure compatiblity.

Caution when using yarn link: Given that at application compile time, the angular CLI will use ngcc or the Angular linker to modify the JS files from the built SDK to make it compatible with the application's Angular version, symlinking to the built JS will change the files there. This will become problematic when testing with multiple Angular versions as compiler errors will emerge. Hence, it is in this case easier to use yarn install ./path/to/tgz/file which will actually install the library in the applications node_modules folder.

fixes #3282
fixes #4644
ref: https://getsentry.atlassian.net/browse/WEB-622
ref: https://getsentry.atlassian.net/browse/WEB-852

[AbhiPrasad]

quick pass through, when this is properly ready to review lmk and I'll dive in!

[Lms24]

Added some information and still open question to the PR description

[lobsterkatie]

A few questions about the PR description (which overall is very thorough and helpful - nice job!):

A more detailed explanation on the compiler differences is given in #4641.

Is this meant to point somewhere else, or is this copy pasta from somewhere else pointing to here?

npm run build builds APF conform JS with the Angular compiler to ./dist

I think this should now be ./build rather than ./dist, right?

npm run pack creates a tgz archive from the APF build in ./dist

Same here?

Also, the repo uses yarn as our package manager/script runner, so probably more idiomatic to s/npm run/yarn.

[lobsterkatie]

3. Can and should we adapt the "one main build directory" strategy from the Angular compiler in our build structure of the other SDKs? It would make the overview over which directories/files are actually packaged much clearer, especially if we're going to unify the type definitions to remove the redundant definitions in esm and dist.

Yes. I've added it to my list of changes.

[lobsterkatie]

4. The Angular compiler prints a warning when including scripts in the package.json that should be published, due to a vulnerability. For the Angular SDK this is easily fixable with a flag in the ng-package.json config to simply exclude scripts from the generated package.json. Can/should we exclude them?

Yes. No reason why consumers of the SDK need our scripts.

[lobsterkatie]

I can't speak to the angular-specific bits, but overall this looks pretty reasonable.

[Lms24]

Is this meant to point somewhere else, or is this copy pasta from somewhere else pointing to here?

Whoops, this should have linked to #4644. Changed it.

I think this should now be ./build rather than ./dist, right?

Yes, fixed.

Also, the repo uses yarn as our package manager/script runner, so probably more idiomatic to s/npm run/yarn.

Right, also fixed.

I can't speak to the angular-specific bits, but overall this looks pretty reasonable.

Cool, thanks for the review!

[Lms24]

Corrected some yarn commands in PR description (yarn pack --> yarn run pack)

[lobsterkatie]

Corrected some yarn commands in PR description (yarn pack --> yarn run pack)

Have you found yarn pack not to work? Generally the run is optional.

[Lms24]

Have you found yarn pack not to work? Generally the run is optional.

I thought so, too but yarn pack will pack the tarball from the directory it's called whereas calling yarn run pack will execute the pack command specified in package.json. This command is not equal to yarn pack anymore as it points to the package.json in build

[lobsterkatie]

Have you found yarn pack not to work? Generally the run is optional.

I thought so, too but yarn pack will pack the tarball from the directory it's called whereas calling yarn run pack will execute the pack command specified in package.json. This command is not equal to yarn pack anymore as it points to the package.json in build

Ohhhhh. Because yarn pack is an actual thing. I think we should rename the script in that case, to avoid confusion (and to make yarn newScriptName work).

[Lms24]

I agree, another name for the script would be great (thinking about yarn package, yarn tarball` or something similar...) to clear up the confusion. I think this should go into its own issue/task though as it requires changes in many (all?) of our JS SDKs and probably some adjustments in our CI/craft config.

[github-actions]

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[Lms24]

un-stale

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	20.23 KB (+0.81% 🔺)
@sentry/browser - ES5 CDN Bundle (minified)	64.37 KB (-0.28% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.85 KB (+0.41% 🔺)
@sentry/browser - ES6 CDN Bundle (minified)	58.03 KB (+0.13% 🔺)
@sentry/browser - Webpack (gzipped + minified)	23.4 KB (+0.82% 🔺)
@sentry/browser - Webpack (minified)	81.29 KB (-1.41% 🔽)
@sentry/react - Webpack (gzipped + minified)	23.45 KB (+0.84% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified)	47.99 KB (-0.03% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	26.07 KB (+0.35% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	24.43 KB (+0.11% 🔺)

[Lms24]

Revisited and updated this PR so that it can be merged into 7.x. Additionally applied some cleanup and removed the legacy options as we decided to not release a legacy package for the time being.