Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(angular): mention TraceDirective bug and workaround in Angular SDK #4780

Merged
merged 4 commits into from
Mar 1, 2022

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Mar 1, 2022

This PR adds a warning alert to tracehelpers.mdx in the Angular SDK guide to make users
aware that the usage of TraceDirective will currently most likely cause a compiler error
on their end. The alert links to the original issue (getsentry/sentry-javascript#3282) and our intended fix (getsentry/sentry-javascript#4644) which will ship with the v7 major of the JS/Angular SDK.

The TraceDirective option was moved down from the top option to the last one as it is essentially unusable
due to the compiler error. The other two options are suggested as workarounds until the bug is fixed and released in the next major.

Additionally, the PR fixes the suboptimal usage instruction of TraceDirective: Instead of declaring TraceDirective in the users' Angular module(s), TraceModule (which wraps the directive) should in fact be imported in the module(s). This conforms to the way how Angular components or directives from libraries should be used.

Add a warning alert to `tracehelpers.mdx` in the Angular SDK guide to make users
aware that the usage of `TraceDirective` will most likely cause a compiler error
on their end. The alert links to the original issue and our intended fix which
will ship with the v7 major of the JS/Angular SDK.

Fix suboptimal usage instruction of `TraceDirective`: Instead of declaring
`TraceDirective` in the users' Angular module(s), `TraceModule` (which wraps the
directive) should in fact be imported in the module(s). This conforms to the way
how Angular component libraries should be used.
@Lms24 Lms24 requested a review from lobsterkatie March 1, 2022 10:54
@vercel
Copy link

vercel bot commented Mar 1, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sentry/sentry-docs/AiX93sGtdyrCmyBbrvLaKLU6taxS
✅ Preview: https://sentry-docs-git-lms-angular-tracedirective.sentry.dev

@Lms24 Lms24 requested a review from imatwawana March 1, 2022 12:11
Copy link
Contributor

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Made some style and wording adjustments, but otherwise looks great (pending technical review)!

…s.mdx

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
Lms24 and others added 2 commits March 1, 2022 15:57
…s.mdx

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
…s.mdx

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@Lms24
Copy link
Member Author

Lms24 commented Mar 1, 2022

Thanks for the reviews!

@Lms24 Lms24 merged commit ef41fda into master Mar 1, 2022
@Lms24 Lms24 deleted the lms-angular-tracedirective branch March 1, 2022 15:27
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants