-
-
Notifications
You must be signed in to change notification settings - Fork 322
feat: use GOPACKAGESDRIVER to load templ documents lazily #1124
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
Conversation
Thanks folks! I'm looking to test this out. Are you using a bespoke GOPACKAGESDRIVER? Or the Bazel driver, I think it's bespoke because the Bazel driver won't return templ files in |
We have a bespoke |
This reverts commit ea8198e.
We wrote a short wrapper around the Bazel driver to allow you to test these changes. It essentially invokes the existing Bazel GOPACKAGESDRIVER, captures its JSON output, and injects any Templ files found in the same directories as the Please let us know if you have any questions. We appreciate you taking the time to review this! |
Oh, that's ideal. I'll take a look at this at some point this week. |
All the work done so far does look good, and I'm confident that you folks have done some thorough testing! I'm just struggling to get it working in my setup. For transparency I'm not a frequent user of bazel so am having to modify an existing app to work with bazel, and also get my setup working with it. I'll keep working at it since I'm confident it's my environment that is causing it to not work. |
Thank you for your time! Would you be willing to share the project you're working with? If you are, we can also double-check on our end. |
I've just worked up a modified version of an example app I use to test new features of templ with that uses bazel: I'm using neovim and have set the following on the templ lsp: I have logged the result of opening a templ file the hovering a symbol.
Interestingly if I open a Go file in the same package it then seemst to load the templ file in properly, and the hover then works! |
I think maybe either before this point we need to convert to using the Go URI, or do it within the lazy loader? Because I think what is happening is the Does your internal GOPACKAGESDRIVER implementation handle templ files as input, is that maybe the difference? |
You're absolutely right — thank you for catching that! I was trying to distill some of our internal code down to a minimal viable product, and I must’ve removed a bit too much in the process. I'm not sure how it got through when I was testing it out on my end, but that was clearly an oversight on my part. Our internal GOPACKAGESDRIVER does handle Templ files correctly as input. I've now updated the shim to rewrite |
Success! That works a treat. Thanks for the help on that one, I managed to learn a bit about bazel in the process. @a-h Just wanted to give you an opportunity to look at this one before I merge. It's quite a niche feature so I thought not documenting it in the main website might be alright for now. I also would've suggested some LSP e2e tests, but I think the complexity there would be high, we would probably need to create a custom GOPACKAGESDRIVER. |
I was wondering if it would be acceptable for us to create a "Used by" section in our README and include Ubers logo on there please? We're proud to have built something picked up by such a notable oranization like Uber. Also am curious if you have any more detail you are allowed to share on what it's used for at Uber if you are comfortable sharing to quench my curiosity :D ? |
We really appreciate all the work y'all have put into Templ — it's been a great tool for us. We’re currently working with our Open Source Program Office to understand what we’re able to share publicly about our use of the project. We’d be happy to provide more context if possible — just want to ensure we’re aligned internally first. I’ll follow up once we have more clarity. @a-h In the meantime, are there any next steps you need from us for this PR? |
Thanks! Yeah fully understand that, thanks for looking into it for us. The plan is to cut a release and then merge this in along with a few other PRs, so will be merged in the next few days :) |
Thank you for your patience! I followed up internally about your request. Regarding the use of Templ at Uber, the company utilizes a range of technologies to support both web server and client-side functionality, including but not limited to Go and server-side rendered HTML. Templ is one of several technologies incorporated within this broader tech stack, and should not be interpreted as our central or exclusive solution for these purposes. That said, Uber would prefer to be listed as both a notable contributor and a user. The company has made some key improvements to support large monorepos and would appreciate being included in a "Notable Contributors" or similar section if you're planning to add one. On behalf of the team at Uber, we really appreciate the collaboration and all the great work you’ve done on this project. We’re looking forward to contributing more going forward! |
Implement lazy loading and unloading of templ documents based on package dependency information provided by a custom
GOPACKAGESDRIVER
, ensuring correct open/close order using topological traversal.The custom
GOPACKAGESDRIVER
must populate the OtherFiles field with all direct and transitive.templ
file dependencies for each Package. It must also include the Templ LSP generated_templ.go
file in the GoFiles and CompiledGoFiles fields to satisfy Gopls.A custom
GOPACKAGESDRIVER
may not be necessary if #1128 can be resolved.Closes #1123.