winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.53k stars 174 forks source link

Implicitly typed extern expression #6045

Open MarkMcCulloh opened 1 month ago

MarkMcCulloh commented 1 month ago

Use Case

Javascript/Typescript for inflight handlers. There is currently a lot of ceremony involved in using extern as a way to consume JS/TS. When doing this, it's assumed that the user knows the types they'll have available when calling the external file. This is cumbersome because it's more typical to call the inflight when consuming APIs that define the types for you.

Ideally, the source of truth for types should be within wing but declared when used. If this is the case, then why not go all the way and implicitly determine the type for the extern.

This use case is different from https://github.com/winglang/wing/issues/6042, where unequivocally the source of truth is the external file itself.

Here's an example of the minimum solution to call a typescript function inflight:

bring cloud;

class Extern {
  pub static extern "./oncreate.ts" inflight onCreate(req: cloud.ApiRequest): cloud.ApiResponse;
}

let bucket = new cloud.Bucket();
bucket.onCreate(inflight (req) => {
  // wrapper function needed because static functions can't be passed as-is (bug)
  return Extern.onCreate(req);
})

Proposed Solution

Introduce a global function or new syntax for "closure loaded from file". This syntax will be an expression that loads an external file, like our current extern implementation. The desired type contract the file is expected to implement is based on the expression's inferred type.

(syntax is a placeholder)

bring cloud;

let bucket = new cloud.Bucket();
bucket.onCreate(extern("./oncreate.ts"))

// error: extern "./oncreate.ts" already has type `(event: OnCreateEvent): void`
bucket.onUpdate(extern("./oncreate.ts"))

Compiling this will generate ./oncreate.extern.d.ts like the current extern behavior, using the type that Bucket.onUpdate expects as a default export. The .ts file can then be implemented like so:

// oncreate.ts
import extern from "./oncreate.extern.d.ts";

export const handler: extern = async (key, type) => {
  console.log(`File Created: ${key}`);
}

export default handler

The return type of extern() is inferred, which means it must be resolved in the same scope. It also must resolve to a function type.

// error: unable to infer type of extern
extern("./oncreate.ts")

// error: extern from be a function
let x: str = extern("./oncreate.ts")

Implementation Notes

We may still want a way to explicitly type the extern. We could reuse the generics syntax:

let func = extern<inflight (): void>("./handler.ts")

Examples

Inflight Handler

bring cloud;

let api = new cloud.Api();

api.get("/", extern("./handler.ts"));
// handler.ts
import extern from "./handler.extern.d.ts";

export const handler: extern = async (req) => {
  console.log(req.path);
  return {
    status: 200
  }
}

export default handler

Lifting

bring cloud;

let bucket = new cloud.Bucket();
let api = new cloud.Api();

api.get(inflight (req) {
  lift { bucket: ["put"] } {
    return extern("./handler.ts")(req, bucket);
  }
});
// handler.ts
import extern from "./handler.extern.d.ts";

export const handler: extern = async (req, bucketClient) => {
  console.log(req.path);
  bucketClient.put("hi", "hello");
  return {
    status: 200
  }
}

export default handler

Preflight

interface UrlHelpers {
  parseURL: (url: str): Json;
  formatURL: (url: Json): str;
}

let helpers: UrlHelpers = extern("./url-helpers.ts");

Community Notes

eladb commented 1 month ago

@MarkMcCulloh would it be useful to shove this into an RFC doc so it will be easier to discuss?

eladb commented 1 month ago

It's actually a way to introduce global externs:

let parseURL: (url: str): Json = extern("./url-helpers.ts").parseURL;

// or
interface UrlHelpers {
  parseURL: (url: str): Json;
  formatURL: (url: Json): str;
}

let helpers: UrlHelpers = extern("./url-helpers.ts");

// or
let MAX_MEMORY: num = extern("./consts.ts").MAX_MEMORY;

I like this - it makes extern much more robust.

Chriscbr commented 1 month ago

Javascript/Typescript for inflight handlers, there is currently a lot of ceremony involved in using extern. When doing this, it's assumed that the user knows the types they'll have available when calling the external file. This is cumbersome because it's more typical to call the inflight when consuming APIs that define the types for you.

I don't feel like I fully understand the use cases. Could you give some concrete examples of the situations you're thinking about? (See for example this RFC where there's examples of Wing code that works today and the issues with it). It's easier to discuss solutions when we have examples of the problem to reference.

MarkMcCulloh commented 1 month ago

would it be useful to shove this into an RFC doc so it will be easier to discuss?

It would, but I wasn't sure how controversial the implicit approach is here so this issue is kinda serving as a pre-rfc-rfc.

Under which export will this go? Can you include the .ts/.js file in the examples above? Is this the default export (module.exports) or an export of some symbol (exports.foo)?

Updated description. I think default export would simplify the implementation greatly and it covers the primary usecase ("run this ts function for me").

And then what would be the syntax for non-default?

I would propose only allowing default exports. Mostly to avoid implementation complications, but also to just generally simplify the usage of this to "this file does one thing" which is the primary use case for this proposal (unlike https://github.com/winglang/wing/issues/6042)

It feels like this shouldn't be restricted to functions. Why wouldn't let x: str = extern("./my-const.ts") work?

I constrained it here mostly to limit scope to the main use case, but I think it would be ok. I'll remove the limitation

Chriscbr commented 1 month ago

Thanks - I think after the extra details were added I feel like I understand the proposal better.

If this is the case, then why not go all the way and implicitly determine the type for the extern.

My only nit is it's worth being clear what's meant by "implicitly" here -- that the type is only filled in based on type inference from surrounding Wing source code (i.e. inference based on where the function is being called). "Implicitly" determining the external function's type by other means, like by parsing the TypeScript code itself, is out of scope for now since it would introduce a lot of complexity into the compiler and impact compile times significantly. With that made clear, I think the rest of the design looks alright to me.


As an aside, from a prioritization perspective, I worry a bit whether we might be focusing too much on "how can we make it easier for users to escape hatch out of Wing as soon as possible" instead of "how do we make the language as convenient to write and bring existing code into."

In the first kind of use case, I'm bringing existing JS/TS code into Wing so I can re-use it as easily as possible. This use case is the better DX for the "99% developer" crowd, since you only have to learn one language - Wing. We're still missing a few pieces to make this story convenient, like automatically generating Wing bindings for TypeScript libraries.

In the second kind of use case, I'm trying to minimize how much Wing code I write by only using Wing for creating infrastructure in preflight, while writing as much of my inflight runtime code in TypeScript as possible. This use case is only worthwhile to users/teams that already know TypeScript and are comfortable requiring new engineers to learn two languages, since it implies a much more hybrid codebase where Wing and TypeScript is mixed together (or even where it's 90%+ TypeScript -- in which case, Wing isn't providing much value outside of the simulator).

eladb commented 1 month ago

As an aside, from a prioritization perspective, I worry a bit whether we might be focusing too much on "how can we make it easier for users to escape hatch out of Wing as soon as possible" instead of "how do we make the language as convenient to write and bring existing code into."

I don't see this as an escape hatch but as an adoption enabler. This is about making it easy and natural for users to adopt Wing in an existing typescript-based project.

Chriscbr commented 1 month ago

(context: I admit I'm totally derailing this issue / just looking to see if there are any good language design tenets I can extract from this discussion - sorry @MarkMcCulloh :D)

Sure, yeah I think I'm in agreement. Putting terminology aside, I only meant to say if our primary goal for Wing is for it to become a general purpose programming language, then it probably makes sense to prioritize the needs of users who are using Wing as the host/primary language of their project(s) over the needs of users using Wing more in the capacity as a build tool or add-on, should these sets of needs should ever come into conflict.

Naturally if there are features that benefit both groups, or which don't compromise the experience of using Wing as a host/primary language -- like the feature proposed in this issue -- then there's no downsides to adding them as far as I can tell.

eladb commented 1 month ago

like the feature proposed in this issue

👍

I would say that a very important design tenant at this stage of the language is to prioritize capabilities that reduce the barrier for people to be able to adopt Wing in existing projects and within their existing workflows :-)

MarkMcCulloh commented 1 week ago

I've been looking into this and I'm thinking about better focusing the goal of the change proposed here. The primary use case this issue is meant to solve are ones like:

bring cloud;
let bucket = new cloud.Bucket();
let queue = new cloud.Queue();
bucket.onCreate(<external type-safe code with lifted access>)

latching this on to extern makes sense because it is our existing mechanism. The problem with extern is that it's very general purpose, used for any type/phase/location. This makes it feel like you're solving a problem with a workaround (like unsafeCast) rather than using a purpose-built tool provided by the toolchain. In practice it also means we're lacking constraints to guide the design to better fit the use case.

So instead, let's have something that can only create inflight functions while in preflight:

// main.w
bring cloud;
let bucket = new cloud.Bucket();
let queue = new cloud.Queue();
bucket.onCreate(@inflight("./handler.ts", lifts: ["queue.push"]))
// handler.ts
import inflight from "./handler.inflight.ts"; // note, no longer .d.ts. Still usable by js.

// you call into the generated code that has the types/clients
export default inflight(async ({ queue }, event) => {
   await queue.push(event);
});

New syntax: @intrinsic

This syntax tells the user that this code has special interactions with the compiler. It isn't a normal value. This gives us freedom to handle things in special ways, like relative paths or dynamic types. This type of syntax would already make sense for some of our current builtins (like @unsafeCase) and maybe some new ones (e.g. @path(), @file(), @dirname, etc. for referencing relative paths). This feature does not technically require a new syntax to implement, but I think it's worthwhile to explore the direction.

Why @?

lifts: ["queue.push"]

The new-ish lift syntax is useful but the ergonomics do not fit with external inflights because there is no scope to put it. The goals are also not aligned because lift is intended to change behavior within actual wing code. I propose a named argument for this @inflight intrinsic that takes an array of strings that reference symbols in scope. This can either be a preflight object itself (giving all permissions) or a specific inflight method. The great part about intrinsics is that we can actually check these values at compile time.

Adding named arguments here is also a

As an aside, @lift would also be a great use of @.

Why not allow within inflight or to return non-function

I think it would technically be possible to implement the following:

bring cloud;
let bucket = new cloud.Bucket();
let queue = new cloud.Queue();
bucket.onCreate(inflight (evt) => {
  let x: num = @inflight("./get_num.ts", lifts: ["queue"])
})

but it adds much more implementation difficulty and I don't think it's a common use case.

eladb commented 1 week ago

I feel like we might be jumping too quickly to adding syntax/builtins to the language and I am wondering if we should try to perhaps identify lower level mechanisms in the compiler/language that will allow us to support this use case through a library instead of built-in compiler support.

I would look into what @eladcon did with bring python and try to generalize this pattern and perhaps offer some more basic compiler capabilities.

Something like:

bring cloud;
bring typescript;

let bucket = new cloud.Bucket();
let queue = new cloud.Queue();

bucket.onCreate(new typescript.inflight.BucketEvent(
  path: "./handler.ts",
  handler: "handle",
  lifts: {
    queue: { obj: queue, ops: ["push"] }
  }
));

Stuff we can/should do at the compiler level that could help:

  1. Offer a way to implement these in through some kind of "baby generics" so there is no need to implement a typescript.XxxHandler for each signature, and then we can maybe have something like typescript.inflight().
  2. Offer a way to generate a .d.ts file for these inflights through some SDK call.

I like the suggestion to use @ for builtins, but I think the topic of how to mark our builtins is important but maybe that's not the right thread to discuss it, so I would extract to another discussion/thread.

MarkMcCulloh commented 1 week ago

@eladb We can feel free to skip the syntax change, it's not a requirement for the idea. The bring python implementation is great but it is fundamentally happening at the wrong stage of compilation, and getting it to have the same access as something builtin would be a huge task. This would effectively require us to have a compiler<->preflight API, which I would adore but I don't see it as worthwhile yet.

Once this is implemented for typescript, @inflight("file.py") would not be a huge addition. I think this greatly strengthens wing as having a model where inflights are just units of compilation.

The example you posted is basically what I'm proposing, where typescript.inflight.BucketEvent is just @inflight where the type is inferred and general ergonomics are much better.

eladb commented 1 week ago

OK, this is growing on me. @inflight("./file.py") would be awesome to be able to support quickly! Worth making sure our design won't break for this use case.

(as an aside, maybe this issue deserves to be consolidated into an RFC... I feel the discussion is getting a bit chaotic and I am personally struggling to capture the entire picture).

The string "queue.push" is meh... Both queue and push are strongly-typed symbols and it feels pretty odd to put them in strings (even if the compiler can check).

It also feels like it might be useful to express the mapping between wing objects and context keys in a more explicit way.

So maybe represent the lifts as a map:

bring cloud;
let bucket = new cloud.Bucket();
let queue = new cloud.Queue();

bucket.onCreate(@inflight("./handler.ts", lifts: {
  my_queue: { lift: queue, ops: ["push"] },
  your_bucket: { lift: bucket, ops: ["put", "list"] },
}));

And then, it could be possible to offer imperative API as well:

bring cloud;
let bucket = new cloud.Bucket();
let queue = new cloud.Queue();

let handler: cloud.IBucketEventHandler = @inflight("./handler.ts");
handler.lift(queue, as: "my_queue", ops: ["push"]);
handler.lift(bucket, as: "your_bucket", ops: ["put", "list"]);

bucket.onCreate(handler);

This is a bit confusing:

let x: num = @inflight("./get_num.ts", lifts: ["queue"]);

Does that mean that get_num.ts exports a number as it's default export? Maybe add the get_num.ts file to the example, because it's not super clear. Also, what's the meaning of lifting queue here in terms of the signature of the default export in this case?

MarkMcCulloh commented 1 week ago

The string "queue.push" is meh... Both queue and push are strongly-typed symbols and it feels pretty odd to put them in strings (even if the compiler can check). It also feels like it might be useful to express the mapping between wing objects and context keys in a more explicit way.

My goal was to avoid the bad (but understandable) ergonomics of existing solutions we have. I think having to state the names of symbols goes against the experience you want most of the time, which is to just use the names you have. This was needed in @wingcloud/framework due to limitations of using typescript, but it's avoidable in wing itself. You mention it's "odd to put them in strings" but with the map we already have to put everything in strings, and { queue: { lift: queue, ops: ["push"] } } is super verbose and repetitive (even with an imperative API).

For the first version of this I'm okay with doing the simple map instead, effectively copying the current lift syntax.

This is a bit confusing: Does that mean that get_num.ts exports a number as it's default export? What's the meaning of lifting queue here in terms of the signature of the default export in this case?

I listed that example as something this feature should not handle. My guess is that yes the number could be a default export and that anything lifted would either be available as a global in the file some accessible anywhere in the file. I don't see a great option though, and I don't think this use case is important enough to consider. My goal with @inflight is basically an extern version of inflight () => {}, so no value is given to non-func values.

(as an aside, maybe this issue deserves to be consolidated into an RFC... I feel the discussion is getting a bit chaotic and I am personally struggling to capture the entire picture).

At this point I'm pretty close to finishing the implementation (I wrote the recent comment after I prototyped enough to see this as feasible), so I figured more discussion can happen there.

eladb commented 1 week ago

I think having to state the names of symbols goes against the experience you want most of the time, which is to just use the names you have.

I totally see what you mean, but there's a difference between optimizing for the common case (which I agree is most likely to reuse the name) and designing the API without an ability to support the non-common case.

I also feel a map-like API will result in more "mechanical empathy" towards the fact that we effectively mapping objects across these domains.

I would recommend starting with the more verbose API that reflects the underlying model a bit better (and also aligned with what we have in TypeScript) and then we can think of "@sugar" to make this more ergonomic. Makes sense?

put everything in strings

It makes sense that the keys are strings but the operations shouldn't be strings. It seems like a temporary solution.

figured more discussion can happen there

FWIW - I am cool with the "@" syntax. It's elegant. But I wouldn't want this to be a one-off. If we decide to use it, then let's use it for all of our builtins.

Maybe even convert test to @test("foo", inflight () => {}) like you originally suggested.

Chriscbr commented 5 days ago

The main aspect of the @inflight() function/macro design that concerns me is that the macro does not enforce a very robust boundary between Wing and TypeScript: the type of the @inflight() expression can only be resolved by type inference, i.e. "what is the type of the hole that the expression is going into". The main purpose of type inference is to improve the ergonomics of the language by reducing the amount of type-annotations needed within function bodies etc. But in this case, the macro depends on type inference to work - which means the user wouldn't get a wing compile error if the underlying type changes, if I understand correctly.

I also think there's merit to see if there's a way to design this to fit more closely with our plans for more ergonomic extern functions, and with the lifting qualification syntax from RFC #5951.


What if we said that whenever there's an extern function definition, the language allows an optional lifts clause for specifying context objects? Suppose this is the Wing file:

// main.w
bring cloud;

let queue = new cloud.Queue();

// since the extern is .js/.ts, Wing will generate vendored.generated.ts
bring extern "./vendored.ts" {
  myConst: str;
  myHandler: cloud.BucketEventHandler;
  myDeleteHandler: cloud.BucketEventHandler lifts { queue: queue };
};

let bucket = new cloud.Bucket();
bucket.onCreate(myHandler);
bucket.onDelete(myDeleteHandler);

note: bring extern "./vendored.ts" could be shortened to extern "./vendored.ts"

The top-level extern clause introduces new variables. The type source of truth is the Wing source file. The extern values can then be used like any other value.

The compiler automatically generates a file with TypeScript types that the developer can optionally use to get IDE typing help and scaffold their TypeScript code, like it does today:

// vendored.ts
import type { extern } from "./vendored.generated";

export const myConst: extern["myConst"] = "59f48c21-c449-456c-822c-b78b14e79fde";

export const myHandler: extern["myHandler"] = async (ctx, key, eventType) => {
    ctx.queue.push(key);
};

export const myDeleteHandler: extern["myDeleteHandler"] = async (key, eventType) => {
    // implementation
};
Note on how this would work in non-entrypoint files
Lift information can also be added to extern methods. This lets users define extern instance methods on classes (not supported today): ```js type DownloadParallel = inflight (keys: Array): Array; // storage.w pub class Storage { bucket: cloud.Bucket; new() { this.bucket = new cloud.Bucket(); // optional qualification reuse mechanism this.downloadQual = std.LiftQualification(this.bucket, allow: ["get"]); } extern "./other.ts" inflight uploadParallel(keys: Array, data: str): void lifts { bucket: { obj: this.bucket, allow: ["put"], } } extern "./other.ts" downloadParallel: DownloadParallel lifts { bucket: this.downloadQual, } } ``` ```ts // other.ts import { handlers } from "./other.generated"; export const uploadParallel: handlers["uploadParallel"] = async (ctx, keys, data) => { await Promise.all(keys.map(key => ctx.bucket.put(key, data))); }; export const downloadParallel: handlers["downloadParallel"] = async (ctx, keys, data) => { return Promise.all(keys.map(key => ctx.bucket.get(key, data))); }; ```
MarkMcCulloh commented 5 days ago

expression can only be resolved by type inference

While this is technically true in the current implementation, it's not the core of the proposal. I think this could easily work just as well:

bucket.onCreate(@inflight<inflight (str): void>("./blah.ts"));

Or, basically the same thing that works in the current implementation:

let handler: cloud.OnCreateHandler = @inflight("./blah.ts")

Does viewing it this way change anything?

The problem with forcing users to provide the type is that it's just a bad experience. It's forcing you to do work that the compiler can do for you. Allowing inference here is the same as allowing inference in the rest of wing: "Just tell me what the types are and I'll get started". The only difference here is that you look at another file after the inference happens.

which means the user wouldn't get a wing compile error if the underlying type changes

By using this with inference you are saying that source of truth for what the type should be is the underlying type itself. So if it changes, then the source of truth has changed too. While yes in the current implementation that wouldn't immediately cause a type error, the hope is that your IDE or any other type checking process from the ts/js side with get the error before runtime. Also I do think it would be ideal for our toolchain to do the additional type checking on the typescript side as well if we had a fast way to do it.

bring extern

I'm not against your proposed bring extern syntax, in fact it would by my preference to get rid of the current extern and switch it to that instead. However I consider the @inflight usecase different and that syntax is way too verbose for it.

cloud.BucketEventHandler

In your example you often make use of these interfaces. This makes the examples simpler but I think it's hiding the real story. Right now our SDK exclusively uses these interfaces to represent inflight handler. However, these are effectively a facade that get converted to an inflight function type. Would you expect the user to have to manually type our the entire function signature every time they wanted to do this?

The examples will often look like this

class MyResource {
  onCreate(handler: inflight (firstArg: str, options: MyStruct): num) { ... }
}

bring extern "./other.ts" {
  // hopefully you have easy access to the original types
  myHandler: inflight (firstArg: str, options: MyStruct): num;
};

You might say "so what?" to that, but this annoyance is one of the problems I set out to solve with this and I don't see how this is better than the current extern "file.ts" syntax.