Skip to content

Conversation

@ryan953
Copy link
Member

@ryan953 ryan953 commented Jul 25, 2024

The toolbar is a little world, we shouldn't import from it (but it's re-using things from all over sentry!)

Also, there's some specific spots that it shouldn't be importing from. Right now just queryClient, but over time that list will probably grow as they get further apart.

The getsentry version of this is https://github.com/getsentry/getsentry/pull/14747

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 25, 2024
@ryan953 ryan953 requested a review from a team July 25, 2024 22:53
@ryan953 ryan953 requested a review from a team July 25, 2024 22:53
@priscilawebdev
Copy link
Member

we have a few places in sentry that use devtoolbars imports. Everything seems to pass now, but I wonder if we will get lint errors after merging this PR. Also, can we add a note to the devtoolbars exports so people see them when trying to import?

@ryan953
Copy link
Member Author

ryan953 commented Jul 29, 2024

we have a few places in sentry that use devtoolbars imports. Everything seems to pass now, but I wonder if we will get lint errors after merging this PR. Also, can we add a note to the devtoolbars exports so people see them when trying to import?

I don't see anything external to the toolbar itself doing an import, so if there is something and linter starts complaining i'll go fix it right away because it shouldn't do that.

I don't think it's going to be practical to annotate all the exports from within the toolbar folder, just because it's any file really that's inside there. I think the most likely thing to get accidentally imported is one of the hooks, specifically if someone is letter vscode choose where to import from. I think i can followup and rename some things so they're less appealing.

I'm mostly just trying to add a layer that helps prevent accidents. I've been saying it for a bit longer than I expected, but this folder is not forever, once we solve some auth related stuff we'll be unblocked and can move this to a new repo.

@ryan953 ryan953 merged commit f39a5a5 into master Jul 29, 2024
@ryan953 ryan953 deleted the ryan953/prevent-devtool-imports branch July 29, 2024 19:08
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants