-
-
Notifications
You must be signed in to change notification settings - Fork 37
feat: add vite-plugin-devtools-json
addon
#581
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: d7129ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Ho WoW, it's linked with sveltejs/svelte#11394 (comment) I will do a PR to fix this. (I will do a separate one, to track things better) |
vite-plugin-devtools-json
addon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your awesome PR. The only thing that I'm unsure about is the case for newly created projects.
Normally I would expect that a newly created project will not throw any errors in the console as this will be very strange for newcomers. On the other hand i don't necessarily want to add code that only helps users with specific browsers. Given that the problem also arises in chromium based browsers like MS Edge we are talking about ~70% market share. I would say that this is not enough to bother everyone with this new tool, so making it an adder sounds like the correct option. We only need to rise awareness about this add-on.
The other thing I'm concerned about is the add-on name vite-plugin-devtools-json
feels clunky and is not telling what it's actually for. We could consider something like chromium-devtools
but I'm sure people will have better ideas than myself.
Edit: As you said, don't bother about the vitest
tests, that's a seperate issue
Co-authored-by: Manuel <[email protected]>
Co-authored-by: Manuel <[email protected]>
Co-authored-by: Manuel <[email protected]>
Co-authored-by: Manuel <[email protected]>
I'm curious, what's your threshold ? Seems pretty high already.
Yes, you are right, it's not that cool! Most of (or at least a lot of) vite plugins starts like this I like your suggestion Let me know, I'll update the PR 👍 |
I don't know, just a feeling. 30% of people might not have that problem. But on the other hand, for us devs, the percentage is potentially totally different.
I think it's fine as is. I was referring to Rich's suggestion to add the plugin directly without user prompt when creating a new project. That's the thing i was considering. That the add-on is visible in the addon selection after
Yeah, totally get your point. I had to refresh my memories because sveltejs/svelte#15910 was once called devtools, but since this is no longer the case we might be good to use Would love to get other opinions on this. |
I would like to raise another option that is turning in my head. Why not having: npx sv add vite-plugin devtools-json
# by default it's:
# 1/ adding devDep: "vite-plugin-devtools-json"
# 2/ importing the plugin in list of vite plugins (with default export camel case `devtoolsJson`) This would allow any* vite plugin to be added! I could see: npx sv add vite-plugin kit-routes -n
# -n for named plugin `import { kitRoutes }...` Feel free to discard the idea :) |
I don't this is a good idea for two reasons:
I think the only thing we need to do here is to decide on the naming. To summarize;
Currently, I would prefer |
Thx for the comment @manuel3108, like this it's out of my head 😉👍 My vote goes for |
I don't have a strong opinion on naming but would be good with Re 70%/30%: market share among the general population isn't really relevant; what matters is the % of developers that use Chromium, and that number is 100% — even those of us who ordinarily use other browsers are still testing our apps in Chromium. Or at least we should be! |
Just got a brillant idea I think! Like this if nothing special, it's in. And if for some reason you want to opt out, you are one space away |
Yeah, that totally makes sense. In that case, we will need to figure out a way to run the |
Yeah just had the same thinking. Feel free to work on this if you want, otherwise I can take a look at it. I think the important thing would be to only make it preselected in the create prompt, not in the add prompt. Also we should probably make this configurable for the add-ons, potentially in the |
I'll play with this tonight 👍 |
Closes: #578
vite-plugin-devtools-json
plugin to vite config[ ] fix: resolve-conditions-for-client #582 needs to be merged before, then tests will pass here.