Skip to content

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

jycouet
Copy link
Contributor

@jycouet jycouet commented Jun 7, 2025

Closes: #578

  • feature: adding vite-plugin-devtools-json plugin to vite config
  • tests (js & ts)
  • doc

Copy link

changeset-bot bot commented Jun 7, 2025

🦋 Changeset detected

Latest commit: d7129ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sv Patch

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

@svelte-docs-bot
Copy link

Copy link

pkg-pr-new bot commented Jun 7, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@581
npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@581

commit: d7129ed

@jycouet
Copy link
Contributor Author

jycouet commented Jun 7, 2025

Ho WoW, it's linked with sveltejs/svelte#11394 (comment)
It's vitest that is failing... and it's failing also in main branch! (and with the current released sv).

I will do a PR to fix this. (I will do a separate one, to track things better)

@manuel3108 manuel3108 changed the title Addon vite plugin devtools json feat: add vite-plugin-devtools-json addon Jun 8, 2025
Copy link
Member

@manuel3108 manuel3108 left a 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

@jycouet
Copy link
Contributor Author

jycouet commented Jun 8, 2025

70% market share. I would say that this is not enough to bother everyone

I'm curious, what's your threshold ? Seems pretty high already.
But yes, if you feel that it's more appropriate, we can remove it from create.
How should I do it, I add a flag in defineAddon like: skipCreate: boolean ? Or Something in setup ?


I'm concerned about is the add-on name vite-plugin-devtools-json

Yes, you are right, it's not that cool! Most of (or at least a lot of) vite plugins starts like this vite-plugin-* and that's a nice convention (pck json organisation for example!)

I like your suggestion chromium-devtools, but it's adding a new term "chromium" and removing one "json", so it would be a "svelte thing". If we plan to add more thing to chromium-devtools, yes, let's go for it. If we don't plan more things we could maybe just call it devtools-json (keeping the author vibe?)

Let me know, I'll update the PR 👍

@manuel3108
Copy link
Member

I'm curious, what's your threshold ? Seems pretty high already.

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.

How should I do it, I add a flag in defineAddon like: skipCreate: boolean ? Or Something in setup ?

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 sv create is totally fine by me.

I like your suggestion chromium-devtools, but it's adding a new term "chromium" and removing one "json", so it would be a "svelte thing". If we plan to add more thing to chromium-devtools, yes, let's go for it. If we don't plan more things we could maybe just call it devtools-json (keeping the author vibe?)

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 devtools-json. But i still se an edge where this could be confusing.

Would love to get other opinions on this.

@jycouet
Copy link
Contributor Author

jycouet commented Jun 8, 2025

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 :)

@manuel3108
Copy link
Member

I don't this is a good idea for two reasons:

  1. our cli args are already complicated another, adding another layer for vite args does not make sense I think.
  2. What would happen if a vite plugin requires more customization than just using it? In that case it would still require a custom adder, with potential for naming collisions.

I think the only thing we need to do here is to decide on the naming. To summarize;

  • vite-plugin-devtools-json feels clunky
  • chromium-devtools adds a new term chromium . Additionally, this also applies for Edge and other chromium based browsers as well
  • devtools-json compact and short

Currently, I would prefer devtools-json.
@Rich-Harris Any opinion on the naming of the add-on, as you initially suggested vite-plugin-devtools-json?

@jycouet
Copy link
Contributor Author

jycouet commented Jun 10, 2025

Thx for the comment @manuel3108, like this it's out of my head 😉👍

My vote goes for devtools-json as well

@Rich-Harris
Copy link
Member

I don't have a strong opinion on naming but would be good with devtools-json.

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!

@jycouet
Copy link
Contributor Author

jycouet commented Jun 10, 2025

Just got a brillant idea I think!
What about selecting it by default in the list ?

Like this if nothing special, it's in. And if for some reason you want to opt out, you are one space away

@manuel3108
Copy link
Member

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!

Yeah, that totally makes sense. In that case, we will need to figure out a way to run the devtools-json add-on right after creating a new project. Or is it potentially enough to pre-select that option in the adder selection prompt after creating the project?

@manuel3108
Copy link
Member

What about selecting it by default in the list ?

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 setup function

@jycouet
Copy link
Contributor Author

jycouet commented Jun 10, 2025

I'll play with this tonight 👍

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.

Add-on for vite-plugin-devtools-json
3 participants