Skip to content

Commit

Permalink
Allow subclass type overrides from string to string-literal (#38)
Browse files Browse the repository at this point in the history
  • Loading branch information
jasongwartz authored Mar 30, 2024
1 parent ace342d commit 27f2faf
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 3 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Until then, minor and patch releases may contain breaking changes.
### Known Current Limitations

- **Inlined imports**: Imported Pkl types are inlined into the output TypeScript file. For example, if `foo.pkl` has an import like `import "bar.pkl"`, and you run `pkl-gen-typescript foo.pkl`, the resulting `foo.pkl.ts` file will include all types defined in `foo.pkl` _as well as_ all types defined in `bar.pkl`. This means that the resulting TypeScript generated files (in a multi-file codegen) will match the set of input root files, not the file structure of the source Pkl files. This behaviour may create unintended name conflicts; these can be resolved using the `@typescript.Name { value = "..." }` annotation. It may also cause duplication (eg. if the same shared Pkl library file is imported in two schemas); TypeScript's structural typing (where equivalent type shapes can be used interchangeably) should mean that any duplicate types can be safely used as each other.
- **Subclass type overrides**: Pkl class definitions are generated as TypeScript interfaces in code generation. However, TypeScript interfaces, when using `extends` to extend a parent type, do not support overriding the type of a property from the parent. Even though this _is_ supported in Pkl subclassing, it is not currently possible to generate TypeScript for Pkl classes that override parent property types.
- **Subclass type overrides**: Pkl class definitions are generated as TypeScript interfaces in code generation; Pkl supports completely changing the type of a property in a child class, but this is not allowed in TypeScript extending interfaces. When a TypeScript interface `extends` a parent interface, overrides of the type of a property must be "compatible" with the parent type (eg. overriding a `string` type with a string-literal type). In TypeScript codegen, overriding a parent string-type property with a string literal type is allowed, and other compatible types may be allowed in the future (if you have an example of where this would be useful, please file a GitHub Issue).
- **Regex deserialisation**: Pkl's `Regex` type will be decoded as a `pklTypescript.Regex` object, which contains a `.pattern` property. Pkl uses Java's regular expression syntax, which may not always be perfectly compatible with JavaScript's regular expression syntax. If you want to use your Pkl `Regex` as a JavaScript `RegExp`, and you are confident that the expression will behave the same way in JavaScript as in Pkl, you can instantiate a new `RegExp` using the `pklTypescript.Regex.pattern` property, eg. `const myConfigRegexp = new RegExp(myConfig.someRegex.pattern)`.
- **IntSeq deserialisation**: Pkl's `IntSeq` type is intended to be used internally within a Pkl program to create a range loop. It is unlikely to be useful as a property type in JavaScript, and is therefore decoded into a custom `pklTypescript.IntSeq` type with signature `{ start: number; end: number: step: number }` - it is _not_ decoded into an array containing the ranged values. If you have a use-case to use `IntSeq` as an array of ranged values in a TypeScript program, please file a GitHub Issue.
- **Duration and DataSize APIs**: Pkl has a rich API for many of its custom types, but two of note (that are not common in standard libraries of other languages) are `Duration` and `DataSize`, which include convenience APIs for eg. converting between units or summing values. These types are decoded into `pklTypescript.DataSize`/`pklTypescript.Duration` types (each of which have a `value` and `unit` property), and do not yet have the convenience APIs from Pkl.
Expand Down
2 changes: 2 additions & 0 deletions codegen/snippet-tests/input/04-withClass.pkl
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ value: MyCustomClass

abstract class MyAbstractClass {
someString: String
overrideableStringType: String
}

class MyConcreteClass extends MyAbstractClass {
anotherString: String
overrideableStringType: "string literal type"
}
4 changes: 4 additions & 0 deletions codegen/snippet-tests/output/04_with_class.pkl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@ export interface MyCustomClass {
// Ref: Pkl class `04-withClass.MyAbstractClass`.
export interface MyAbstractClass {
someString: string

overrideableStringType: string
}

// Ref: Pkl class `04-withClass.MyConcreteClass`.
export interface MyConcreteClass extends MyAbstractClass {
anotherString: string

overrideableStringType: "string literal type"
}

// LoadFromPath loads the pkl module at the given path and evaluates it into a N04WithClass
Expand Down
13 changes: 11 additions & 2 deletions codegen/src/internal/ClassGen.pkl
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ local function isSameType(typeA: reflect.Type, typeB: reflect.Type) =
// but we will pretend it is always false for now.
else false

// TypeScript allows extending interfaces to override properties from the parent interface as long
// as the new type is compatible with the parent property's type. One useful example of this, and
// the only current supported way below, is when the parent type is String-type, and the child type is a string-literal type.
// In the future we could add more detailed compatibility checks.
local function isCompatibleType(parentType: reflect.Type, childType: reflect.Type) =
parentType is reflect.DeclaredType &&
parentType == reflect.stringType &&
childType is reflect.StringLiteralType

// visible for testing
function getFields(
clazz: reflect.Class,
Expand All @@ -78,14 +87,14 @@ function getFields(
// don't render hidden members
if (prop.modifiers.contains("hidden")) false
// Okay if there is no property override, or if the super property has the same type.
else if (superProp == null || isSameType(superProp.type, prop.type)) true
else if (superProp == null || isSameType(superProp.type, prop.type) || isCompatibleType(superProp.type, prop.type)) true
// Okay if the property is overridden but does not define a type, but don't render as its own field.
// E.g. `class Foo extends Bar { bar = "mybar" }`
else if (prop.type is reflect.UnknownType) !isSuperOpen
// Otherwise, the property's type has been overridden, and this is currently
// not supported - would require something like `extends Omit<ParentClass, 'overriddenField'>`
else throw("""
Illegal: Class `\(clazz.reflectee)` overrides property `\(propName)`. This is not supported when generating TypeScript.
Illegal: Class `\(clazz.reflectee)` overrides property `\(propName)`. This is only supported by TypeScript when the new type is compatible with the parent type.
\(prop.location.displayUri)
""")
Expand Down

0 comments on commit 27f2faf

Please sign in to comment.