V3-Alpha-Bugfix bindings reference update #4690
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRemoved exported GitURLToModuleName and go.mod mutation from the init command; introduced an internal gitURLToModuleName and a TemplateOptions.GoModule field, and updated templates and template tests to use and render Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as init command (old)
participant Templates as templates subsystem (new)
participant Renderer as Template Renderer
CLI->>CLI: (old) GitURLToModuleName(url) [exported]
CLI->>CLI: (old) Modify go.mod with derived module
Templates->>Templates: gitURLToModuleName(url) -- derive GoModule --
Templates->>Renderer: pass templateData { GoModule }
Renderer->>Renderer: render templates with {{.GoModule}}
note over Renderer: Frontend templates now import bindings/{{.GoModule}}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
github-actions
bot
added
the
Documentation
Deploying wails with
|
| Latest commit: |
935c04d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://718106d4.wails.pages.dev |
| Branch Preview URL: | https://travis-v3-alpha-bugfix-bindi.wails.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
v3/internal/templates/templates.go (1)
492-522: Consider handling trailing slashes in Git URLs.The
gitURLToModuleNamefunction correctly handles multiple URL formats, but doesn't trim trailing slashes. URLs likehttps://github.com/user/repo.git/or[email protected]:user/repo/would result in module names ending with/.Apply this diff to handle trailing slashes:
// Remove leading forward slash for file system paths moduleName = strings.TrimPrefix(moduleName, "/") + // Remove trailing forward slash + moduleName = strings.TrimSuffix(moduleName, "/") return moduleName }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
v3/internal/commands/init.go(1 hunks)v3/internal/templates/_common/go.mod.tmpl(1 hunks)v3/internal/templates/base/frontend/main.js.tmpl(1 hunks)v3/internal/templates/lit-ts/frontend/src/my-element.ts.tmpl(1 hunks)v3/internal/templates/lit/frontend/src/my-element.js.tmpl(1 hunks)v3/internal/templates/preact-ts/frontend/src/app.tsx.tmpl(1 hunks)v3/internal/templates/preact/frontend/src/app.jsx.tmpl(1 hunks)v3/internal/templates/qwik-ts/frontend/src/app.tsx.tmpl(1 hunks)v3/internal/templates/qwik/frontend/src/app.jsx.tmpl(1 hunks)v3/internal/templates/react-swc-ts/frontend/src/App.tsx.tmpl(1 hunks)v3/internal/templates/react-swc/frontend/src/App.jsx.tmpl(1 hunks)v3/internal/templates/react-ts/frontend/src/App.tsx.tmpl(1 hunks)v3/internal/templates/react/frontend/src/App.jsx.tmpl(1 hunks)v3/internal/templates/solid-ts/frontend/src/App.tsx.tmpl(1 hunks)v3/internal/templates/solid/frontend/src/App.jsx.tmpl(1 hunks)v3/internal/templates/svelte-ts/frontend/src/App.svelte.tmpl(1 hunks)v3/internal/templates/svelte/frontend/src/App.svelte.tmpl(1 hunks)v3/internal/templates/sveltekit-ts/frontend/src/routes/+page.svelte.tmpl(1 hunks)v3/internal/templates/sveltekit/frontend/src/routes/+page.svelte.tmpl(1 hunks)v3/internal/templates/templates.go(3 hunks)v3/internal/templates/templates_test.go(2 hunks)v3/internal/templates/vanilla-ts/frontend/src/main.ts.tmpl(1 hunks)v3/internal/templates/vanilla/frontend/main.js.tmpl(1 hunks)v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl(3 hunks)v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: fbbdev
Repo: wailsapp/wails PR: 4001
File: v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/function_single_internal/greetservice.ts:0-0
Timestamp: 2025-01-15T22:33:30.639Z
Learning: In the Wails framework's TypeScript bindings, the import path "/wails/runtime.js" is intentionally absolute and should not be changed to a relative path.
📚 Learning: 2025-01-15T22:33:30.639Z
Learnt from: fbbdev
Repo: wailsapp/wails PR: 4001
File: v3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/function_single_internal/greetservice.ts:0-0
Timestamp: 2025-01-15T22:33:30.639Z
Learning: In the Wails framework's TypeScript bindings, the import path "/wails/runtime.js" is intentionally absolute and should not be changed to a relative path.
Applied to files:
v3/internal/templates/solid-ts/frontend/src/App.tsx.tmplv3/internal/templates/qwik-ts/frontend/src/app.tsx.tmplv3/internal/templates/react-swc-ts/frontend/src/App.tsx.tmplv3/internal/templates/svelte/frontend/src/App.svelte.tmplv3/internal/templates/sveltekit/frontend/src/routes/+page.svelte.tmplv3/internal/templates/qwik/frontend/src/app.jsx.tmplv3/internal/templates/vanilla-ts/frontend/src/main.ts.tmplv3/internal/templates/preact-ts/frontend/src/app.tsx.tmplv3/internal/templates/react-ts/frontend/src/App.tsx.tmplv3/internal/templates/sveltekit-ts/frontend/src/routes/+page.svelte.tmplv3/internal/templates/svelte-ts/frontend/src/App.svelte.tmplv3/internal/templates/preact/frontend/src/app.jsx.tmplv3/internal/templates/base/frontend/main.js.tmplv3/internal/templates/solid/frontend/src/App.jsx.tmplv3/internal/templates/lit/frontend/src/my-element.js.tmpl
🧬 Code graph analysis (1)
v3/internal/templates/templates.go (2)
v3/internal/commands/init.go (1)
Init(51-115)v3/internal/flags/init.go (1)
Init(3-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (go)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (29)
v3/internal/templates/templates_test.go (2)
1-1: LGTM: Package declaration correctly updated.The package change from
commandstotemplatescorrectly reflects the relocation of the module name derivation logic.
109-109: LGTM: Test updated to use unexported function.The test correctly targets the relocated and now-unexported
gitURLToModuleNamehelper. The comprehensive test coverage (18 test cases covering various git URL formats, protocols, and edge cases) remains intact.v3/internal/commands/init.go (2)
10-11: Import grouping adjusted.The import reorganization is a minor formatting change with no functional impact.
21-49: LGTM: Cleaner separation of concerns confirmed.The
initGitRepositoryfunction now focuses solely on git operations (initializing repo, creating remote, staging files) without modifyinggo.mod. This is a cleaner design as module naming responsibility has moved to the templating system.Verification confirms
templates.Installproperly:
- Derives
GoModulefrom the-gitflag viagitURLToModuleName()(line 262)- Falls back to project name when derivation returns empty (lines 263-265)
- Passes
GoModuleto template rendering inTemplateOptionsstruct (line 269)The
gitURLToModuleName()function handles HTTPS, SSH, Git protocol, and filesystem URLs correctly, with test cases validating behavior across GitHub, GitLab, BitBucket, and custom domains.v3/internal/templates/qwik/frontend/src/app.jsx.tmpl (1)
2-2: LGTM: Import path correctly templatized.The change from
../bindings/changemeto../bindings/{{.GoModule}}correctly implements dynamic binding resolution for the Qwik template.v3/internal/templates/solid-ts/frontend/src/App.tsx.tmpl (1)
2-2: LGTM: Import path correctly templatized.The change from
../bindings/changemeto../bindings/{{.GoModule}}correctly implements dynamic binding resolution for the Solid TypeScript template.v3/internal/templates/react-swc/frontend/src/App.jsx.tmpl (1)
2-2: LGTM: Import path correctly templatized.The change from
../bindings/changemeto../bindings/{{.GoModule}}correctly implements dynamic binding resolution for the React SWC template.v3/internal/templates/base/frontend/main.js.tmpl (1)
1-1: LGTM: Import path correctly templatized.The change from
./bindings/changemeto./bindings/{{.GoModule}}correctly implements dynamic binding resolution for the base vanilla JavaScript template. The relative path./(versus../in other templates) is intentional, reflecting thatmain.jsis at thefrontend/root rather than in asrc/subdirectory.v3/internal/templates/sveltekit/frontend/src/routes/+page.svelte.tmpl (1)
2-2: LGTM: Import path correctly templatized.The change from
../../bindings/changemeto../../bindings/{{.GoModule}}correctly implements dynamic binding resolution for the SvelteKit template. The deeper relative path../../is intentional, reflecting SvelteKit's file-system routing structure where this component is nested insrc/routes/.v3/internal/templates/_common/go.mod.tmpl (1)
1-1: Module name validation not implemented—verify this limitation is acceptable.The
GoModulefield is properly populated with fallback logic:
- Lines 262-265 in
templates.go: callsgitURLToModuleName(options.Git)and falls back tooptions.ProjectNameif empty- This correctly handles missing
-gitflag and empty URLsHowever, no validation exists for Go module naming constraints. The
gitURLToModuleNamefunction (lines 494-522) strips URL schemes and normalizes formatting but does not validate that the result is a valid Go module name (lowercase alphanumeric + dots/slashes/hyphens only). Malformed input could produce invalid module names that fail duringgo mod tidyexecution rather than failing early.Confirm whether this deferred validation strategy (relying on
go mod tidyto catch errors) is acceptable, or if upfront module name validation should be added.v3/internal/templates/react-swc-ts/frontend/src/App.tsx.tmpl (1)
2-2: LGTM! Dynamic module binding correctly implemented.The import path now uses the
{{.GoModule}}template variable instead of the hardcoded "changeme" placeholder, enabling proper binding resolution when projects are initialized with custom Git URLs.v3/internal/templates/svelte/frontend/src/App.svelte.tmpl (1)
2-2: LGTM! Import path correctly templated.The binding path now dynamically resolves to the correct Go module at generation time.
v3/internal/templates/vanilla-ts/frontend/src/main.ts.tmpl (1)
1-1: LGTM! Binding path correctly updated.The dynamic module binding will resolve properly when the template is rendered.
v3/internal/templates/preact/frontend/src/app.jsx.tmpl (1)
2-2: LGTM! Template variable correctly applied.The import path now uses
{{.GoModule}}for dynamic binding resolution.v3/internal/templates/vanilla/frontend/main.js.tmpl (1)
1-1: LGTM! Correct relative path for frontend root.The import uses
./bindings/(not../bindings/) which is appropriate since this file is in thefrontend/root directory rather thanfrontend/src/.v3/internal/templates/qwik-ts/frontend/src/app.tsx.tmpl (1)
2-2: LGTM! Binding path templated correctly.Consistent with other framework templates in using
{{.GoModule}}for dynamic module resolution.v3/internal/templates/react-ts/frontend/src/App.tsx.tmpl (1)
2-2: LGTM! Dynamic binding correctly implemented.The template now properly resolves the Go module binding at generation time.
v3/internal/templates/preact-ts/frontend/src/app.tsx.tmpl (1)
2-2: LGTM! Import path correctly updated.Verification confirms that
{{.GoModule}}is properly defined in theTemplateOptionsstruct, consistently populated during template rendering, and used across 22 template files. All frontend templates now correctly reference dynamic module bindings instead of hardcoded values.v3/internal/templates/react/frontend/src/App.jsx.tmpl (1)
2-2: LGTM! Import path correctly updated to use dynamic module.The change from hardcoded "changeme" to
{{.GoModule}}resolves the binding reference issue described in the PR objectives.v3/internal/templates/sveltekit-ts/frontend/src/routes/+page.svelte.tmpl (1)
2-2: LGTM! Import path correctly updated with appropriate relative depth.The use of
../../bindings/{{.GoModule}}is correct for this file's location insrc/routes/.v3/internal/templates/lit-ts/frontend/src/my-element.ts.tmpl (1)
3-3: LGTM! Import path correctly updated.The dynamic
{{.GoModule}}placeholder will resolve to the correct binding path at template generation time.v3/internal/templates/svelte-ts/frontend/src/App.svelte.tmpl (1)
2-2: LGTM! Import path correctly updated.The change is consistent with other frontend templates in this PR.
v3/internal/templates/lit/frontend/src/my-element.js.tmpl (1)
2-2: LGTM! Import path correctly updated.v3/internal/templates/solid/frontend/src/App.jsx.tmpl (1)
2-2: LGTM! Import path correctly updated.v3/internal/templates/templates.go (2)
78-78: LGTM! New field enables dynamic module binding resolution.The
GoModulefield allows templates to reference the correct binding path, solving the core issue described in the PR objectives.
262-269: LGTM! Correct fallback logic for module name derivation.The logic appropriately derives the module name from the Git URL when available, falling back to the project name otherwise.
v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl (1)
3-3: LGTM! Import path correctly updated.The change to
../../bindings/{{.GoModule}}is correct for this file's location.v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl (2)
3-3: LGTM! Import path correctly uses template variable.The change from hardcoded "changeme" to
{{.GoModule}}fixes the bindings reference issue described in the PR objectives. This ensures generated projects use the correct module path from the start.
35-47: LGTM! Vue template syntax correctly escaped for Go template processing.The escaped curly braces (
\{\{ \}\}) are necessary to prevent Go's template engine from interpreting Vue's interpolation syntax. During generation,\{\{ msg \}\}becomes{{ msg }}in the final Vue component, which Vue then processes correctly at runtime.
v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl
Outdated
Show resolved
Hide resolved
| <div><p>\{\{ time \}\}</p></div> | ||
| </div> | ||
| </template> | ||
| ∑ |
There was a problem hiding this comment.
Remove stray character at end of file.
The "∑" character appears to be unintentional and should be removed.
Apply this diff:
</template>
-∑📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ∑ | |
| </template> |
🤖 Prompt for AI Agents
In v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl
around line 48, there is a stray "∑" character at the end of the file; remove
that character and ensure the file ends cleanly (no unintended characters) and
preferably ends with a single newline.
| @@ -1,6 +1,6 @@ | |||
| <script setup> | |||
| ©<script setup> | |||
There was a problem hiding this comment.
Critical: Remove stray "©" character before <script setup>.
The "©" character will break Vue's Single File Component syntax, causing parse errors during frontend builds.
Apply this diff to fix:
-©<script setup>
+<script setup>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ©<script setup> | |
| <script setup> |
🤖 Prompt for AI Agents
In v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl around
lines 1 to 1, there is a stray "©" character immediately before the opening
"<script setup>" tag which breaks Vue SFC parsing; remove the leading "©" so the
file begins directly with "<script setup>" (no extra characters or whitespace
before the tag) and save the template.
tmclane
force-pushed
the
travis-v3-alpha-bugfix/bindings-reference
branch
from
4a05f66 to
007adab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl (1)
1-1: Critical: The stray "©" character must be removed.This issue was previously flagged and remains unresolved. The "©" character before
<script setup>will cause Vue SFC parse errors and break frontend builds.Apply this diff:
-©<script setup> +<script setup>
🧹 Nitpick comments (1)
v3/internal/templates/templates_test.go (1)
5-5: Consider renaming test function for consistency.The test function name
Test_GitURLToModuleNamestill references the old exported function name, but now tests the unexportedgitURLToModuleNamefunction. Consider renaming toTest_gitURLToModuleNameorTestGitURLToModuleNamefor better alignment with the tested function.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
v3/UNRELEASED_CHANGELOG.md(2 hunks)v3/internal/templates/templates_test.go(2 hunks)v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl(3 hunks)v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- v3/internal/templates/vue-ts/frontend/src/components/HelloWorld.vue.tmpl
- v3/UNRELEASED_CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (go)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
v3/internal/templates/vue/frontend/src/components/HelloWorld.vue.tmpl (2)
3-3: LGTM: Import path fix correctly addresses the PR objective.The change from hardcoded
changemeto the templated{{.GoModule}}ensures that bindings are correctly referenced based on the actual Go module path when-gitis provided duringwails3 init.
35-47: LGTM: Vue template interpolations are correctly escaped.The Go template escaping (
{{"{{"}}/{{"}}"}})) properly preserves Vue's mustache syntax in the generated output, ensuring that{{ msg }},{{ result }}, and{{ time }}render correctly in the final Vue component.v3/internal/templates/templates_test.go (1)
1-1: LGTM: Package name correctly updated.The package declaration change from
commandstotemplatescorrectly reflects the relocation of the test to align with the internalgitURLToModuleNamefunction now residing in the templates package.
|
Until I get full testing in here I am converting it to 'draft'. |
|
Description
This MR corrects a small annoyance when using the
initcommand while providing a-gitargument.In that case the generated Javascript bindings in the
frontend/src/bindingsdirectory are not placed in thechangemedirectory anymore.This causes a compilation failure that must be corrected by editing one file per generated project to add in the proper path to the generated bindings.
See the attachment for the complete testing done.
testing.md
For example:
Produces
The culprit is :
The generated bindings are found in :
Fixes # (issue)
Type of change
Please select the option that is relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.If you checked Linux, please specify the distro and version.
Test Configuration
Summary by CodeRabbit