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 for Capacitor 5/Angular 16 peer dependencies error #458

Closed
wants to merge 3 commits into from

Conversation

ahunter135
Copy link
Contributor

No description provided.

@afilp
Copy link

afilp commented Sep 7, 2023

@lucas-zimerman Can you please review and merge this? As we need to do an npm install and without this change it fails.

Thanks a lot!

@lucas-zimerman
Copy link
Collaborator

In this case wouldn't it break for uses using the non ivy version of Angular?

@ahunter135
Copy link
Contributor Author

@lucas-zimerman

It might. I'm new to this. But we also need to do an install and it fails without this.

Side effect of this change: I'm on capacitor angular, but I can't import it from there. I have to import from sentry/angular-ivy. So there could be more to do here.

I just wanted to start the discussion back up because at this point, sentry is not supporting capacitor 5 and angular 16.

@emelampianakis
Copy link

emelampianakis commented Sep 7, 2023

angular 16 needs the ivy distribution. if this is the case maby you can remove the dependency completely so each dev can install the package needed.
Sentry suggests ivy for angular 11+

@ahunter135
Copy link
Contributor Author

So instead of putting ivy in, just remove it entirely? I can do that. @emelampianakis

@emelampianakis
Copy link

emelampianakis commented Sep 8, 2023

So instead of putting ivy in, just remove it entirely? I can do that. @emelampianakis

Yeah, I think this is the correct approach, but then the Installation part of the readme should also be updated.
Maby like this from https://docs.sentry.io/platforms/javascript/guides/angular/?original_referrer=https%3A%2F%2Fwww.google.com%2F

#Angular 12 and newer:
npm install --save @sentry/capacitor @sentry/angular-ivy

#Angular 10 and 11:
npm install --save @sentry/capacitor @sentry/angular

@lucas-zimerman you think this would be ok?

@emelampianakis
Copy link

Hello @ahunter135 can you try the changes I mentioned above?
Just remove both @sentry/angular & @sentry/angular-ivy dependencies and update the readme file.
We really need this fix cause as it is right now angular 16 projects cant use this package.

@ahunter135
Copy link
Contributor Author

@emelampianakis made the requested changes!

@afilp
Copy link

afilp commented Sep 15, 2023

@lucas-zimerman Can you please approve and merge the changes so that we can start using the fix soon (with Angular 16)? Thanks a lot!

@krystofwoldrich
Copy link
Member

Based on #405 (comment) the peer deps are not the issue.

Thank you, everyone, for the comments, please use @sentry/angular-ivy with Angular 12 and newer.

Follow up PR with readme update from this PR -> #469

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants