Skip to content
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

Allow FTL to support external types by utilising type widening #1296

Closed
Tracked by #1612
alecthomas opened this issue Apr 18, 2024 · 1 comment · Fixed by #2010
Closed
Tracked by #1612

Allow FTL to support external types by utilising type widening #1296

alecthomas opened this issue Apr 18, 2024 · 1 comment · Fixed by #2010
Assignees

Comments

@alecthomas
Copy link
Collaborator

alecthomas commented Apr 18, 2024

In the following example did.PortableDID is a type from an external library. Currently we disallow this, but this pattern occurs a lot, particularly when dealing with open standards, etc.

Currently we need to do something like this:

var portableDID = ftl.Secret[map[string]any]("did_web_portable_did")

// Access the DID

func bearerDID(ctx context.Context) did.BearerDID {
	m := portableDID.Get(ctx)
	data, err := json.Marshal(m)
	if err != nil {
		panic(err)
	}
	var d did.PortableDID
	if err = json.Unmarshal(data, &d); err != nil {
		panic(err)
	}
	out, err := did.FromPortableDID(d)
	if err != nil {
		panic(err)
	}
	return out
}

Which is represented in the schema like so:

secret did_web_portable_did {String: Any}

Gross.

The proposal is that because there's no type safety anyway, we allow external types to be used directly and the FTL type extraction would find a wider type that would still represent the concrete type in Go. In this case it would be a {String: Any}, but for eg. an array of something it might be [Any].

var portableDID = ftl.Secret[did.PortableDID]("did_web_portable_did")

That is, it would still be represented in the FTL schema as {String: Any} but would automatically decode into a strong Go type. This is no less type safe than what we already have, and in some ways is more type safe because at least we have strong typing in Go. In other languages such as Kotlin the same pattern could apply, using the custom external types for that "type".

Once #1285 lands, if type widening occurs a warning would be issued.

This would also apply to request/response types, etc. Basically anywhere FTL types are used.

Potential solution

Here's a potential proposal @matt2e , @gak and @alecthomas discussed:

Problems:

  1. Making this sane cross-language.
  2. Making it simple to use.

Proposal:

Introduce type aliases so that type mapping annotations can be added to the named types.

typealias DID {String: Any}
  +typemap go "github.com/foo/bar.DID"
  +typemap kotlin "com.tbd.did.PortableDID"

secret did_web_portable_did DID

Then in Go this might look like the following.

//ftl:typealias
//ftl:typemap kotlin "com.tbd.did.PortableDID"
type DID did.PortableDID

// UserID represents a user ID.
//
//ftl:typealias
type UserID string

var portableDID = ftl.Secret[did.PortableDID]("did_web_portable_did")

In this example we've defined a type alias "DID", which gives FTL a named type to add annotations to, and because we know the target type we can allow users to implicitly use the "real" external type and we will take care of the type mapping for them.

Longer term in a multi-language world we would want the ability for both producers and consumers to be able to override these type mappings.

@github-actions github-actions bot added the triage Issue needs triaging label Apr 18, 2024
@alecthomas alecthomas changed the title Allow FTL types to be relaxed when using external types Allow FTL types to be widened when using external types Apr 18, 2024
@alecthomas alecthomas changed the title Allow FTL types to be widened when using external types Allow FTL to support external types by utilising type widening Apr 18, 2024
@alecthomas alecthomas added the RFC Request for Comment label Apr 18, 2024
@alecthomas alecthomas changed the title Allow FTL to support external types by utilising type widening RFC: Allow FTL to support external types by utilising type widening Apr 18, 2024
@alecthomas
Copy link
Collaborator Author

There is another ticket re. mapping to/from external types

@alecthomas alecthomas added next Work that will be be picked up next and removed triage Issue needs triaging labels Apr 19, 2024
deniseli added a commit that referenced this issue May 7, 2024
This PR:
* Rips out the existing textMarshaler and jsonMarshaler usage from
encoding.go. We may want to add those back for
#1296 down the road, but we
will need to be thoughtful about how we do that. Removing it for now
keeps the logic much more predictable.
* Moves the (un)marshaling logic for `ftl.Option` out of `option.go` and
into `encoding.go`.
* Special-cases both `time.Time` (the only stdlib type we currently
support) and `ftl.Option`. Also `json.RawMessage` for _just_ encoding to
preserve the existing `omitempty` behavior.
* Fixes some existing issues where the Pointer unmarshaling wasn't
actually working correctly
* [eww] Adds a rather grotesque alternative to `Peek()` in
`isNextTokenNull()` because json Decoder does not support Peek.
* [eww] Makes the ftl.Option struct fields public so that they are
settable by `encoding.go`.

Suggestions welcome for both counts of [eww] above :)

Fixes #1247.
Addresses most of #1262, except
`omitempty` is only working for json.RawMessage for now.
@alecthomas alecthomas removed the RFC Request for Comment label May 12, 2024
@alecthomas alecthomas changed the title RFC: Allow FTL to support external types by utilising type widening Allow FTL to support external types by utilising type widening May 31, 2024
@github-actions github-actions bot removed the next Work that will be be picked up next label Jun 4, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
worstell added a commit that referenced this issue Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants