#dang: escaping shadowing (aka `self` or `import`?)

1 messages · Page 1 of 1 (latest)

jade thorn
jade thorn
#

hmm although technically maybe 1 should also be qualifying types, like DAG.Container - that's a bit less nice (...but still i guess less typing?)

jade thorn
#

Laying things out more clearly since this thread was created late.

Problem scenario:

type MyType {
  pub container: Container! {
    base.withExec(["foo"])
  }
  let base: Container! {
    # boom
    container.from("golang")
  }
}

Basically right now it's very easy to accidentally shadow a root-level function, and in the above example it leads to infinite mutual recursion => stack overflow => 💥

Solution 1: explicit import and qualified usage

import Dagger

type MyType {
  pub container: Dagger.Container! {
    base.withExec(["foo"])
  }
  let base: Dagger.Container! {
    Dagger.container.from("golang")
  }
}

Solution 2: require explicit self. to refer to sibling fields

type MyType {
  pub container: Container! {
    self.base.withExec(["foo"])
  }
  let base: Container! {
    container.from("golang")
  }
}

Honestly swaying between both choices moment by moment, both have pros/cons

cc @steep coyote @rich sluice if you have any thoughts 🙂

steep coyote
rich sluice
#

Yeah sounds good to me. I was surprised at first that I could do container. without a top level client

jade thorn
#

So do I read that right as @steep coyote in favor of 2 and @rich sluice in favor of 1?

#

re: importing Dagger, in the broader context of Dang the idea is to also support importing any GraphQL API, e.g. you could have a GitHub Dagger module that uses both APIs natively

rich sluice
#

oh i guess so 🙈 2 works for me too though now that i think about it

jade thorn
#

going option 2 doesn't shut the door on that of course, just option 1 would make Dagger consistent with the rest, but there could also be like import * from Dagger as today's semantics

#

(the trickiest part is credentials, even just to introspect GitHub's API for example you need a token, so you'd need that at typechecking time)

#

in other words there's a chance import is ultimately not feasible and it's more of like 'dang runtime config' mapping GraphQL APIs to qualified imports (or 'import all')

rich sluice
#

is there a 3rd option where we error if we try to reassign a variable/function that we've imported, or are the consequences of that bad

jade thorn
#

mm yeah it seems common enough to want to define a Container() method on module objects so I don't think that's in the cards

jade thorn
#

Landed on an Option 3, and liking it so far.

Similar to option 1, there's an import:

import Dagger

Only, you don't have to qualify every usage site. You only need to when there's a conflict, like if you import two things that provide the same Type, symbol, or @directive. 99% of these modules are probably just going to import Dagger, so it's nice that we don't have to sacrifice ergonomics to support the possibility of the other 1%.

So, to resolve the original issue, you just change container to Dagger.container in the spot where you want to call the original, and leave all the other code unchanged.

Unfortunately it's not easy to turn the original problem into a typechecking error instead of a runtime stack overflow, because there are legitimate cases where you want to shadow the outer scope. I think turning shadowing into an error is probably too brittle, since it would mean any newly added toplevel APIs - which tend to be common short nouns (file, json, module) - would cause type errors in any modules that used those as fields or local vars (which also tend to be common short nouns).

PR: https://github.com/vito/dang/pull/9

cc @white mirage

white mirage
#

option 3 makes sense to me (we can always change it later if we change our minds after dogfooding)

HOWEVER I want to make the case against entangling this with imports. I think what we want for option 3 is actually a special symbol to represent the root query. I think this is orthogonal to imports.

Candidates for the root symbol:

  • _, eg _.container()
  • $, example $.container()
  • leading ., example .container()
  • $root, _root...

IMO this is better because it doesn't put undue pressure on imports. Schema stitching is a cool feature and I support it. But dang is an embedded language which means there will always be a contextual schema that devs expect to be there, without requiring an import. When embedded in Dagger that context is the core dagger schema + current dependencies; in another system it will be something else, but there will still be a contextual schema and developers will still expect it to be there within requiring an import.

Specifically, requiring import Dagger makes the dang/dagger DX worse, and we don't actually need it to solve dang's disambiguation problem.

white mirage
#

also it would free you to make graphql imports a regular builtin:

let ghapi = import_graphql("api.github.com/...")

jade thorn
# white mirage also it would free you to make graphql imports a regular builtin: `let ghapi = ...

Imports have to be special because they introduce new types. When you do import GitHub, that doesn't just bring in a typed GitHub value, it also brings in a GitHub type for qualifying other types when needed (GitHub.Repository). Or, if you wanted to pass GitHub itself around, you could. You wouldn't be able to qualify as ghapi.Repository in your example because that's mixing typechecking and runtime phases, and the whole point of typechecking is to prevent a faulty runtime. And you wouldn't be able to pass around values that came out of the API, because there'd be no way to refer to them. (If I put my thinking cap on there might be some way to get this to work but it'd be swimming upstream, and we'd have even bigger problems if someone expects to be able to pass an expression value as an argument, which seems very likely.)

re: importing by URL "api.github.com/..." - it looks cool in theory, but the reason imports are a bare word like Dagger or GitHub is to decouple the script from the specifics of how that API is provided (e.g. API endpoint and credentials - think GitHub enterprise). I don't think it's wise to tightly couple individual files to those details, especially considering a Dang package might be split into multiple files that use the same imports (Go style). I haven't planned how to map GitHub to an endpoint yet, but have some loose ideas. (For context, I started with import "api.github.com" and moved away from it for all of these reasons.)

Just adding color to this point specifically for now, chewing on the idea of a root-level getter and have some concerns but not discarding it yet. Strawman counter-proposal is to bind the Query type so you can explicitly Query.container but that may be too GraphQL-coded. My main concern is that a token like _ or $ or $root will be cryptic and less guessable than just referring back to the one-liner import name. (For example: what exactly does it refer to? Is it the top-level module scope, including my own bindings, or is it Query?) Yes technically the import is poorer DX insofar as you have to write a two-word line of code that you otherwise might not need to, but seems just about as minimal of a DX papercut as you can get, comparable to having to learn what the special root getter is and know that it exists.

white mirage
#

  • imports needing to be specifial -> ah of course! forget that part 😁

  • `import "api.github.com..."' makes sense, I just assumed that's how it worked, not particularly proposing that format

jade thorn
#

sorry for mostly attacking the followup thought and not the crux 😛 - intent was to context dump for now