-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
defualt to embed all:frontend/dist #3887
defualt to embed all:frontend/dist #3887
Conversation
WalkthroughThe pull request updates the changelog to document new features, changes, and bug fixes in the project. Key additions include new commands for asset management, enhancements to the application package, and improved support for Windows options. The embedding directive for frontend assets has been modified to include all files in the specified directory. Several bugs have been addressed, particularly for macOS and Linux. Overall, the changes reflect a significant enhancement in functionality and user experience across various platforms. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
changelog.md
4e73761
to
07c2da3
Compare
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
v3/internal/templates/_common/main.go.tmpl (1)
Line range hint
11-14
: Update comments to reflect subdirectory embedding.The existing comments should be updated to mention that files in subdirectories under
frontend/dist
will also be embedded.// Wails uses Go's `embed` package to embed the frontend files into the binary. -// Any files in the frontend/dist folder will be embedded into the binary and +// Any files in the frontend/dist folder and its subdirectories will be embedded into the binary and // made available to the frontend. // See https://pkg.go.dev/embed for more information.mkdocs-website/docs/en/changelog.md (1)
Line range hint
93-94
: Remove duplicate changelog entries.The following entries appear twice in the changelog:
- "Do not bind internal service methods in [Ignore internal service methods when binding #3720]"
- "Fixed system tray startup panic in [v3: ShellNotifyIcon fails when shell not loaded yet/properly #3693]"
Please remove the duplicate entries to maintain clarity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
mkdocs-website/docs/en/changelog.md
(1 hunks)v3/internal/templates/_common/main.go.tmpl
(1 hunks)
🔇 Additional comments (1)
mkdocs-website/docs/en/changelog.md (1)
32-32
: LGTM! The changelog entry accurately reflects the changes.
The entry clearly documents:
- The modification to asset embedding
- The reason for the change (support for frameworks generating subfolders)
- Proper attribution and PR reference
This aligns well with the PR objectives of ensuring compatibility with SvelteKit templates.
@@ -14,7 +14,7 @@ import ( | |||
// made available to the frontend. | |||
// See https://pkg.go.dev/embed for more information. | |||
|
|||
//go:embed frontend/dist | |||
//go:embed all:frontend/dist |
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.
💡 Codebase verification
The embed directive change requires updates to example files for consistency
The verification revealed that there are two example files still using the old frontend/dist
embed directive:
v3/examples/file-association/main.go
v3/examples/dev/main.go
These should be updated to use all:frontend/dist
for consistency with the template change and to properly support frameworks that generate subfolders.
Note: The v2 test file already uses the all:
prefix, which further validates this approach.
🔗 Analysis chain
LGTM! The embed directive change correctly addresses SvelteKit compatibility.
The addition of the all:
prefix to include subdirectories is the right approach to support frameworks like SvelteKit that generate assets in subdirectories.
Let's verify the impact on binary size and existing templates:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other templates that might be affected by this change
# and analyze typical dist directory structures
# Look for other template files that might need similar updates
echo "Checking for other template files with embed directives:"
rg -l "go:embed.*frontend/dist" --type go
# Look for potential documentation that needs updating
echo "Checking for documentation that might need updates:"
fd -e md -e txt | xargs rg -l "frontend/dist"
Length of output: 856
Script:
#!/bin/bash
# Let's examine the embed directives in the found files to understand their usage
echo "Checking embed directive in file-association example:"
rg -A 5 -B 2 "go:embed.*frontend/dist" v3/examples/file-association/main.go
echo -e "\nChecking embed directive in dev example:"
rg -A 5 -B 2 "go:embed.*frontend/dist" v3/examples/dev/main.go
echo -e "\nChecking embed directive in v2 test:"
rg -A 5 -B 2 "go:embed.*frontend/dist" v2/internal/staticanalysis/test/standard/main.go
# Let's also check if there are any other similar embed patterns we should consider
echo -e "\nChecking for other embed patterns with frontend:"
rg "go:embed.*frontend" --type go
# Let's examine the documentation mentions to understand the context
echo -e "\nChecking frontend/dist mentions in runtime docs:"
rg -A 2 -B 2 "frontend/dist" mkdocs-website/docs/en/learn/runtime.md
echo -e "\nChecking frontend/dist mentions in changelog:"
rg -A 2 -B 2 "frontend/dist" mkdocs-website/docs/en/changelog.md
Length of output: 2553
Changes the template to use
all:frontend/dist
instead offrontend/dist
added sveltekit templates were not compatible due to the lack of/dist/<subfolder>
Summary by CodeRabbit
New Features
wails3 update build-assets
andwails3 generate runtime
.InitialPosition
option for window positioning.Bug Fixes
AlwaysOnTop
feature on macOS and compilation errors on Linux.Documentation