Skip to content

fix: remove unnecessary dependencies to promises and @sentry/wizard#180

Closed
olivier-blanc-openit wants to merge 1 commit intogetsentry:mainfrom
olivier-blanc-openit:dependencies-cleanup
Closed

fix: remove unnecessary dependencies to promises and @sentry/wizard#180
olivier-blanc-openit wants to merge 1 commit intogetsentry:mainfrom
olivier-blanc-openit:dependencies-cleanup

Conversation

@olivier-blanc-openit
Copy link
Copy Markdown

Sentry/capacitor had a runtime dependency on Sentry/wizard, which then depends on rxjs through inquirer.

This cause issues in monorepo configurations, for instance : ReactiveX/rxjs#6628 and webpack/webpack#14405.

Since Sentry/wizard is more or less a CLI tool, it is not required as a runtime dependency.

@lucas-zimerman
Copy link
Copy Markdown
Collaborator

wizard may be required once #39 is implemented.

@bruno-garcia
Copy link
Copy Markdown
Member

bruno-garcia commented Jun 21, 2022

wizard may be required once #39 is implemented.

That also sounds like a dev dependency. I believe the OP's point still stands.

That said I recall we removed it once on React Native, and it caused issue.

getsentry/sentry-react-native#2015

The PR linked doesn't say what error it was throwing, so it's unclear to me if this would break things here too. Maybe @jennmueng (author of the PR) or @marandaneto (approver) have additional thoughts

@jennmueng
Copy link
Copy Markdown
Contributor

@bruno-garcia The issue with getsentry/sentry-react-native#2015 was that the linking script for RN on iOS automatically runs the Sentry Wizard, which wouldn't exist and would fail.

As we don't do any auto linking scripts here AFAIK that use the wizard we should be good to remove it.

@bruno-garcia
Copy link
Copy Markdown
Member

@olivier-blanc-openit could u please add a changelog entry?

@bruno-garcia
Copy link
Copy Markdown
Member

Also, I believe the wizard should become a dev dependency then, no?

@kopax-polyconseil
Copy link
Copy Markdown

Hello, is there a way to hack this up with patch-package so we don't ship @sentry/wizard with our .... webapp?

Yes it's true, this react-native module affect our web bundle , see getsentry/sentry-react-native#2409

@marandaneto marandaneto requested review from kahest and removed request for bruno-garcia October 31, 2022 09:11
@lucas-zimerman
Copy link
Copy Markdown
Collaborator

An update: Sentry Wizard will be supported by Capacitor, the promises package will be removed from the dependencies and Sentry Wizard will be moved to the devDependencies. All that after the integration with the Wizard is completed

@github-actions
Copy link
Copy Markdown
Contributor

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 🥀

@marandaneto
Copy link
Copy Markdown
Contributor

An update: Sentry Wizard will be supported by Capacitor, the promises package will be removed from the dependencies and Sentry Wizard will be moved to the devDependencies. All that after the integration with the Wizard is completed

Should we reopen this issue? The stale bot closed it.

@mikolajwilczek
Copy link
Copy Markdown
Contributor

mikolajwilczek commented Jun 26, 2023

Hello! What's the status of this issue? npm audit --omit=dev throws 3 critical vulnerabilities because of that dependencies.

wizard may be required once #39 is implemented.

I don't think we should keep things "for the future". Moreover, we can always restore dependencies from the git history

PS: I can create new PR if needed

@lucas-zimerman
Copy link
Copy Markdown
Collaborator

This PR will move to #400
Thank you @olivier-blanc-openit for the contribution!

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.

8 participants