Inconsistency with toTyped and class instantiation #83

New Issue

adam · 2025-12-30T01:20:32+01:00

adam commented

2025-12-30 01:20:32 +01:00

Originally created by @Senjai on GitHub (Feb 19, 2024).

Consider

class Bird {
  name: String
  lifespan: Int
}

// Allowed
pigeon = new Dynamic {
  name = "Pigeon"
  lifespan = 8
  notInBird = "not in bird"
}.toTyped(Bird)

// Not Allowed
eagle = new Bird {
  name = "Eagle"
  lifespan = 8
  notInBird = "not in bird"
}

How do I make the allowed example fail with an error?

toTyped is more lenient than class instantiation, and I'm not sure that should be the case. It might be a good default, since dynamic data can be well, dynamic and I can understand why we wouldn't want people to have to remove a bunch of non relevant data if they are working with truly dynamic data. That said, in cases like this, or when validating JSON which you want to be strongly typed with no leaked data, we are missing a toStrictTyped or toTyped with a boolean flag to ensure that it behaves as if it was defined in Pkl.

Something like this is needed in my opinion both to ensure the normal course of converting from dynamic to typed can behave without surprises (e.g. toMap().remove(x).toTyped) and for validating external schemas or data generated by other tools, since we might not want superflous data in those sources

Originally created by @Senjai on GitHub (Feb 19, 2024). Consider ```pkl class Bird { name: String lifespan: Int } // Allowed pigeon = new Dynamic { name = "Pigeon" lifespan = 8 notInBird = "not in bird" }.toTyped(Bird) // Not Allowed eagle = new Bird { name = "Eagle" lifespan = 8 notInBird = "not in bird" } ``` How do I make the allowed example fail with an error? `toTyped` is more lenient than class instantiation, and I'm not sure that should be the case. It might be a good default, since dynamic data can be well, dynamic and I can understand why we wouldn't want people to have to remove a bunch of non relevant data if they are working with truly dynamic data. That said, in cases like this, or when validating JSON which you want to be strongly typed with no leaked data, we are missing a `toStrictTyped` or `toTyped` with a boolean flag to ensure that it behaves _as if_ it was defined in Pkl. Something like this is needed in my opinion both to ensure the normal course of converting from dynamic to typed can behave without surprises (e.g. `toMap().remove(x).toTyped`) and for validating _external_ schemas or data generated by other tools, since we might not want superflous data in those sources

adam commented

2025-12-30 01:20:32 +01:00

@holzensp commented on GitHub (Feb 19, 2024):

You could define (using import "pkl:reflect"):

function expectedKeys(clazz: reflect.Class?): Set<String> = if (clazz == null) Set() else
  clazz.properties.keys + expectedKeys(clazz.superclass)

function hasAdditionalFields(object: Dynamic, clazz: Class): Boolean =
  let (props = expectedKeys(reflect.Class(clazz)))
    object.toMap().keys.every((it) -> props.contains(it) || throw("Unexpected property \(it)"))

and constrain it through that, but there is no built-in mechanism for it.

@holzensp commented on GitHub (Feb 19, 2024): You could define (using `import "pkl:reflect"`): ``` function expectedKeys(clazz: reflect.Class?): Set<String> = if (clazz == null) Set() else clazz.properties.keys + expectedKeys(clazz.superclass) function hasAdditionalFields(object: Dynamic, clazz: Class): Boolean = let (props = expectedKeys(reflect.Class(clazz))) object.toMap().keys.every((it) -> props.contains(it) || throw("Unexpected property \(it)")) ``` and constrain it through that, but there is no built-in mechanism for it.

adam commented

2025-12-30 01:20:32 +01:00

@Senjai commented on GitHub (Feb 19, 2024):

You could define (using import "pkl:reflect"):

Thank you. I saw some of this in the deepToTyped package too, I'll note it down to dig into this.

there is no built-in mechanism for it

Should there be? Curious to hear thoughts there. If this is possible as written, combined with the deepToTyped, it should be possible to contribute this to pkl-pantry I presume?

My argument for having it be built-in is to address the principle of least surprise. Outside of Dynamic objects, Pkl seems to want to ensure that you instantiate or amend your data in a way consistent with the type being referenced. Maybe that's not a real argument because on reflection I do think the current behavior is still probably best as default behaviour.

@Senjai commented on GitHub (Feb 19, 2024): > You could define (using import "pkl:reflect"): Thank you. I saw some of this in the deepToTyped package too, I'll note it down to dig into this. > there is no built-in mechanism for it Should there be? Curious to hear thoughts there. If this is possible as written, combined with the deepToTyped, it should be possible to contribute this to pkl-pantry I presume? My argument for having it be built-in is to address the principle of least surprise. Outside of Dynamic objects, Pkl seems to want to ensure that you instantiate or amend your data in a way consistent with the type being referenced. Maybe that's not a real argument because on reflection I do think the current behavior is still probably best as default behaviour.

adam commented

2025-12-30 01:20:33 +01:00

@holzensp commented on GitHub (Feb 20, 2024):

Layers of “built-in.” For sure there is a place for this in pkl-pantry. I was thinking about being built-in in the language, and the problem there is that there are a few more constraints.

For example, Pkl is (quite) lazy. To do a properly deep type cast, requires forcing everything. This is also why toTyped is shallow. Thinking about properties existing that are not in the type, in terms of (lazy) evaluation order; when should that error surface? These are tricky questions that need some proper laying out, before we can roll anything into the language.

@holzensp commented on GitHub (Feb 20, 2024): Layers of “built-in.” For sure there is a place for this in `pkl-pantry`. I was thinking about being built-in in the language, and the problem there is that there are a few more constraints. For example, Pkl is (quite) lazy. To do a properly deep type cast, requires forcing everything. This is also why `toTyped` is _shallow_. Thinking about properties existing that are not in the type, in terms of (lazy) evaluation order; when should that error surface? These are tricky questions that need some proper laying out, before we can roll anything into the language.

adam commented

2025-12-30 01:20:33 +01:00

@bioball commented on GitHub (Feb 22, 2024):

For example, Pkl is (quite) lazy. To do a properly deep type cast, requires forcing everything. This is also why toTyped is shallow. Thinking about properties existing that are not in the type, in terms of (lazy) evaluation order; when should that error surface?

I actually don't think deep implies eager. We should be able to fix toTyped so that it will cast recursively, but keep laziness, unless I'm missing something here.

@bioball commented on GitHub (Feb 22, 2024): > For example, Pkl is (quite) lazy. To do a properly deep type cast, requires forcing everything. This is also why toTyped is shallow. Thinking about properties existing that are not in the type, in terms of (lazy) evaluation order; when should that error surface? I actually don't think deep implies eager. We _should_ be able to fix `toTyped` so that it will cast recursively, but keep laziness, unless I'm missing something here.

adam referenced this issue

2025-12-30 01:24:18 +01:00

[PR #83] [MERGED] Only build static executable if musl toolchain is installed #402

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/pkl#83

Inconsistency with toTyped and class instantiation #83

Inconsistency with `toTyped` and class instantiation #83