feat(v3/bindings): add support for binding standard time.Time type as JS Date object#5398
Open
fbbdev wants to merge 6 commits into
Open
feat(v3/bindings): add support for binding standard time.Time type as JS Date object#5398fbbdev wants to merge 6 commits into
time.Time type as JS Date object#5398fbbdev wants to merge 6 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds configurable support to map Go time.Time to JS Date or string in bindings: TimeType flag, SpecialTypeCache, system-path/time resolution, generator validation, render/create adjustments, runtime DateFromTime, example updates, and testdata for multiple JS/TS modes. Changestime.Time Binding Conversion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
v3/examples/binding/GreetService.go (1)
35-36: 💤 Low valueConsider timezone implications of
Local()formatting.The code formats the birthday using
Local(), which converts to the server's timezone. For user-facing dates (especially birthdays), the displayed date might differ from what the user expects if the server is in a different timezone.For production code, consider either:
- Formatting on the client side (JavaScript has timezone-aware formatting)
- Using UTC consistently:
person.Birthday.UTC().Format("2 Jan 2006")- Including timezone information in the output
This is acceptable for an example, but worth noting for real-world applications.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/examples/binding/GreetService.go` around lines 35 - 36, The code uses person.Birthday.Local() which converts the date to the server timezone; update the formatting to avoid server-timezone surprises by using a timezone-consistent representation — e.g., replace person.Birthday.Local().Format("2 Jan 2006") with person.Birthday.UTC().Format("2 Jan 2006") or include explicit timezone info (e.g., Format with time.RFC3339 or append " UTC") in the greeting construction inside the GreetService (the code that builds greeting using person.Birthday) so displayed birthdays are not shifted by server locale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/examples/binding/assets/index.html`:
- Around line 112-113: The hard-coded birthday uses local-time construction
which causes timezone shifts when serialized; update the call to
GreetService.GetPerson to pass a UTC-based Date (e.g., construct with Date.UTC)
so the calendar date is preserved across timezones—replace new Date(1947, 0, 8)
with a UTC construction (new Date(Date.UTC(1947, 0, 8))) when invoking
GreetService.GetPerson.
In `@v3/internal/generator/errors.go`:
- Line 16: The error variable ErrNoContextPackage contains an unnecessary
trailing space in its message; update the string in ErrNoContextPackage to
remove the trailing space so the message ends with "...('context')" and then
scan the same file for the other error variables referenced in the review (the
error message lines at 21, 26, 31, and 35) and remove any trailing spaces from
their literal strings as well to keep messages consistent.
In
`@v3/internal/generator/testdata/output/lang`=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.js:
- Around line 9-11: The typedefs TimeAliasStruct and TimeStruct are being
emitted as "any" instead of the configured scalar (string) for types derived
from time.Time; update the emitter logic that generates typedefs for
time-derived types so that when a field or typedef maps to time.Time (affecting
TimeAliasStruct, TimeStruct and usages like TimeFieldStruct) it emits the
configured scalar ("string" in this mode) rather than "any". Locate the code
path that decides the JS typedef/type name for time-based types (the
function/method generating typedefs for alias/struct types and the type resolver
used by the field emitter), add a branch to detect time.Time (or the generator's
time scalar mapping) and return the scalar representation, and ensure both the
typedef declarations and any referenced field types (e.g., TimeFieldStruct) use
that scalar.
In
`@v3/internal/generator/testdata/output/lang`=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.ts:
- Line 109: The map converter for $$createType3 is using
$Create.Map($Create.Any, $Create.Any) but for field M (a map whose values are
time.Time in Date mode) the value converter must be $Create.DateFromTime; update
the binding generator so that when generating the converter for maps with
time.Time values it emits $Create.Map($Create.Any, $Create.DateFromTime) (ensure
the generation logic that produces $$createType3 detects the time.Time value
type in Date mode and substitutes DateFromTime accordingly).
- Line 22: The generated TypeScript field "M" is incorrectly typed as `{ [_ in
string]?: string }` but in Date mode it should use Date for values (i.e. `{ [_
in string]?: Date }`); update the map-type rendering logic in the generator (the
code that emits the TS type for map fields, referenced by the field name "M" and
the map-type code path) so that when Date mode is enabled and the Go map value
is time.Time it emits Date as the value type instead of string.
In
`@v3/internal/generator/testdata/output/lang`=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.ts:
- Around line 78-79: The runtime converters $$createType0 and $$createType1 are
using $Create.Any but must match the declared field types S (string[]) and M
({[_ in string]?: string}); update $$createType0 to use
$Create.Array($Create.String) so array elements are strings, and update
$$createType1 to use $Create.Map($Create.String, $Create.String) so map
keys/values are typed as strings; make the changes where $$createType0 and
$$createType1 are defined to restore type-safe runtime conversion for fields S
and M.
---
Nitpick comments:
In `@v3/examples/binding/GreetService.go`:
- Around line 35-36: The code uses person.Birthday.Local() which converts the
date to the server timezone; update the formatting to avoid server-timezone
surprises by using a timezone-consistent representation — e.g., replace
person.Birthday.Local().Format("2 Jan 2006") with
person.Birthday.UTC().Format("2 Jan 2006") or include explicit timezone info
(e.g., Format with time.RFC3339 or append " UTC") in the greeting construction
inside the GreetService (the code that builds greeting using person.Birthday) so
displayed birthdays are not shifted by server locale.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b596ea1-c17a-4353-8460-9951d50e1ac4
📒 Files selected for processing (47)
v3/examples/binding/GreetService.gov3/examples/binding/README.mdv3/examples/binding/assets/bindings/github.com/wailsapp/wails/v3/examples/binding/data/models.jsv3/examples/binding/assets/bindings/github.com/wailsapp/wails/v3/examples/binding/greetservice.jsv3/examples/binding/assets/index.htmlv3/examples/binding/data/person.gov3/internal/commands/build_assets/Taskfile.tmpl.ymlv3/internal/flags/bindings.gov3/internal/generator/collect/collector.gov3/internal/generator/collect/imports.gov3/internal/generator/collect/special.gov3/internal/generator/collect/void.gov3/internal/generator/config/paths.gov3/internal/generator/errors.gov3/internal/generator/generate.gov3/internal/generator/generate_test.gov3/internal/generator/load.gov3/internal/generator/render/create.gov3/internal/generator/render/default.gov3/internal/generator/render/type.gov3/internal/generator/testcases/time/bound_types.jsonv3/internal/generator/testcases/time/main.gov3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.jsv3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.jsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=false/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=false/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/index.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.tsv3/internal/generator/testdata/output/lang=TS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/service.tsv3/internal/runtime/desktop/@wailsio/runtime/src/create.ts
💤 Files with no reviewable changes (1)
- v3/internal/generator/collect/void.go
Comment on lines
+9
to
+11
| /** | ||
| * @typedef {any} TimeAliasStruct | ||
| */ |
Contributor
There was a problem hiding this comment.
time.Time-derived typedefs are still emitted as any
Line 10 and Line 29 keep TimeAliasStruct/TimeStruct as any, which undermines the new time.Time mapping behavior and leaks weak typing into downstream APIs (e.g., TimeFieldStruct). These should resolve to the configured scalar (string in this interface-mode output).
Proposed expected output shape
/**
- * `@typedef` {any} TimeAliasStruct
+ * `@typedef` {string} TimeAliasStruct
*/
/**
- * `@typedef` {any} TimeStruct
+ * `@typedef` {string} TimeStruct
*/Also applies to: 28-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@v3/internal/generator/testdata/output/lang`=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.js
around lines 9 - 11, The typedefs TimeAliasStruct and TimeStruct are being
emitted as "any" instead of the configured scalar (string) for types derived
from time.Time; update the emitter logic that generates typedefs for
time-derived types so that when a field or typedef maps to time.Time (affecting
TimeAliasStruct, TimeStruct and usages like TimeFieldStruct) it emits the
configured scalar ("string" in this mode) rather than "any". Locate the code
path that decides the JS typedef/type name for time-based types (the
function/method generating typedefs for alias/struct types and the type resolver
used by the field emitter), add a branch to detect time.Time (or the generator's
time scalar mapping) and return the scalar representation, and ensure both the
typedef declarations and any referenced field types (e.g., TimeFieldStruct) use
that scalar.
Contributor
Author
There was a problem hiding this comment.
This is a very hard problem. Will add a detailed comment about the issue.
Contributor
There was a problem hiding this comment.
@fbbdev, thanks for flagging this — that context will be very helpful. I can see why it's non-trivial: TimeStruct (struct { time.Time }) and TimeAliasStruct (struct embedding a time.Time alias) are struct wrappers around time.Time, not aliases of it. Their JSON marshaling behavior may differ from bare time.Time depending on whether additional fields exist or whether the promoted methods are sufficient, and in interface mode there's no class/constructor to attach conversion logic to. The any fallback is likely the safe, conservative choice until the right approach is worked out.
Happy to look at this more carefully once you've shared your analysis. Feel free to take your time.
✏️ Learnings added
Learnt from: fbbdev
Repo: wailsapp/wails PR: 5398
File: v3/internal/generator/testdata/output/lang=JS/UseInterfaces=true/UseNames=true/github.com/wailsapp/wails/v3/internal/generator/testcases/time/models.js:9-11
Timestamp: 2026-05-10T18:25:13.625Z
Learning: In the Wails v3 binding generator (v3/internal/generator), struct types that embed `time.Time` (e.g. `TimeStruct struct { time.Time }`) or embed a `time.Time` alias (e.g. `TimeAliasStruct`) are intentionally emitted as `any` in interface mode. Collapsing them to the configured time scalar (`string` or `Date`) is a hard, unsolved problem because determining whether a wrapper struct is semantically equivalent to `time.Time` requires deep structural analysis, and their JSON marshaling behavior may differ. The `any` fallback is a deliberate conservative choice.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Contributor
Author
|
Re: @coderabbitai 's observation here. type EmbedsTime struct {
time.Time
}maps to: type EmbedsTime = any:Simple type-level matching is not enough to work around this issue; one should instead retrieve the method set of the type, check whether the MarshalJSON method is present and identical to This is doable, but at the cost of a significant increase in complexity, and IMO out of scope for this PR. If we decide to pay this price, we might as well go all the way and enable people to specify their own deserialization logic for custom marshaler types (which would be a nice addition). |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Your proposed approach of comparing the Would you like me to open a GitHub issue to track this limitation and the idea of supporting custom deserialization logic for custom marshaler types? ✏️ Learnings added
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for mapping the standard Go
time.Timetype to aDateobject in generated bindings.To avoid breaking existing apps, Time-to-Date translation is disabled by default and only enabled through the addition of a new flag
-time-type=Date.When the flag is not provided, or the binding generator runs in interface mode,
time.Timevalues map to the JSstringtype rather thananyas before.The taskfile template has been updated so that newly created projects will be automatically opted in to the new behaviour. Users have the choice to opt out by editing their taskfile.
Fixes #4753
NOTE: No documentation update is required because the docs already state (wrongly) that
time.Timemaps to JSDateobjects.Design choices
-time-type=Temporal. This has been postponed because Temporal objects serialize to JSON as RFC9557 strings (a superset of RFC3339), while Go's marshal/json v1 only supports strict RFC3339.time.Time, but I scrapped it because IMO special casing is harmful and best avoided except in rare and justified cases.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.New and existing binding generator tests verified to pass.
Binding example updated and tested manually.
Test Configuration
Checklist:
(v2 only) I have updatedwebsite/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)I have made corresponding changes to the documentationSummary by CodeRabbit