feat(angular): Add Sentry setup in App Config #769
Conversation
|
| const appConfigFilename = 'app.config.ts'; | ||
| const appConfigPath = path.join( | ||
| process.cwd(), | ||
| 'src', | ||
| 'app', | ||
| appConfigFilename, | ||
| ); |
There was a problem hiding this comment.
hmm I'm a bit concerned that we don't find this file, given it's in src/app/app.config.ts only by convention but afaik not required. Suggestion: let's check if the file exists and add a tag so that we can track hit/miss rates. If the file doesn't exist, we can bail out of this step and link to the setup docs. Depending on the hit/miss rate, we can make our detection logic or fallback behaviour smarter later. WDYT? (cc @andreiborza)
There was a problem hiding this comment.
Just adding to this since I re-checked documentation: I think we can just go with the suggested tracking and bail approach. I don't think this should be a major concern, given that docs don't mention renaming or moving these files. Nevertheless, tracking hit/miss is still helpful
src/angular/sdk-setup.ts
Outdated
| import { updateAppConfigMod } from './codemods/app-config'; | ||
| import type { SemVer } from 'semver'; | ||
|
|
||
| export function hasSentryContent( |
There was a problem hiding this comment.
l (already suggested in #768): let's use the ast-utils version if possible
onurtemizkan
force-pushed
the
onur/angular-setup-app-config
branch
from
6baf639 to
e095e8d
Compare
* feat(angular): Add Sentry setup in App Config * Use `hasSentryContent` from `ast-utils` * Bail out if `app-config` is not under pre-defined location
Supersedes #741 - Part 3
Targeted to the epic branch.
Adds Sentry setup in
app-configfilesLocally tested on Angular 17/19
#skip-changelog