-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(vite-devtools): automatic Chrome DevTools workspace mapping #2925
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
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 so much for the investigation and the implementation 🙏🙏🙏, I hope it wasn't too hard, and that it was fun too!
I have some minor style nitpicks, but I can bring those up later. First, I have two major questions, can you take a look?
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.
I see this code is very similar to the one at https://github.com/ChromeDevTools/vite-plugin-devtools-json.
Are there any major changes? Can we use vite-plugin-devtools-json
as a dependency directly? That code is MIT licensed, so if we copy it in, we should at least find a way to credit it. But it's easier to just use it as a dependency.
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.
yeah its similar , but we added findProjectRoot()
which respects WASP_PROJECT_ROOT when present, otherwise walks up until it finds a .wasp folder, falling back to config.root. This is essential for monorepos and for the generated .wasp/out tree. another thing which i made is pathnormalization
that rewrites Linux paths into UNC form so Chrome can mount them as a “file-system workspace” on Windows hosts – something the upstream plugin doesn’t try to solve. other than those points the file is still ~90 % identical to the MIT-licensed original plugin.
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.
@cprecioso if you say , i will use it as a dependency
|
||
startWebApp :: Path' Abs (Dir ProjectRootDir) -> J.Job | ||
startWebApp projectDir = do | ||
let webAppDir = projectDir </> Common.webAppRootDirInProjectRootDir | ||
runNodeCommandAsJob webAppDir "npm" ["start"] J.WebApp | ||
envVars = [("WASP_PROJECT_ROOT", SP.fromAbsDir projectDir)] |
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.
Not super against this, but have you explored passing this data to the vite.config.ts
template? I think that would be a cleaner design.
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.
@cprecioso yes i explored it but the thing is The absolute project path isn’t known until wasp start runs, and it changes across machines/CI/WSL. Hard-coding it into the generated vite.config.ts would go stale; exporting it at runtime with WASP_PROJECT_ROOT always gives the plugin the correct path.
@0xTaneja I think the improvements you made to the plugin are super nice! Other users of the plugin will also probably want to have them. And we'd prefer to just use the plugin as a dependency if it's so similar. So this is what I would do:
Moreover, I'd like to scrap the environment variable approach, it feels like a way too wide solution for the problem we have. I'd prefer to write that variable into the What do you think? Is this plan nice for you? You'll also get twice the open source contributions from this 🤘 |
@cprecioso the plan sounds perfect !! let's tag team and win this !! |
okay i will start editing vite config template and data associated with it |
oh @0xTaneja I meant to communicate that this is what you should do! this way the contirbutions are on your name! |
@cprecioso understood captain !! will be doing this !! |
@cprecioso, first part is done, pr got merged for chrome devtools, now i can focus on second part of directly using the plugin as a dependency in our Vite config, |
56b9a8d
to
2f20528
Compare
2f20528
to
ec1f3be
Compare
Description
closes #2803
Select what type of change this PR introduces:
Update Waspc ChangeLog and version if needed
If you did a bug fix, new feature, or breaking change, that affects
waspc
, make sure you satisfy the following:ChangeLog.md
with description of the change this PR introduces.waspc
version inwaspc.cabal
to reflect changes I introduced, with regards to the version of the latest wasp release, if the bump was needed.Description
Added Chrome DevTools “Automatic Workspace” support to Wasp apps in dev-mode. here is how i implemented it ,
vite/devToolsJson.ts
plugin that serves/.well-known/appspecific/com.chrome.devtools.json
:wasp-cli
launcher now injectsWASP_PROJECT_ROOT
; if its not present plugin falls back to walking up to thefirst directory containing
.wasp/
for projects started directly with Vite.\\wsl.localhost\<distro>\…
, Docker Desktop).crypto.randomUUID()
, persisted innode_modules/.vite/<hash>/uuid.json
.