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

Add compatibility support for a few more type overrides #41

Merged
merged 4 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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; 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).
- **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). TypeScript codegen currently has support for a few compatible types, and others may be allowed in the future (if you have an example of a compatible type that should work but fails in codegen, 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
12 changes: 12 additions & 0 deletions codegen/snippet-tests/input/04-withClass.pkl
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,21 @@ value: MyCustomClass
abstract class MyAbstractClass {
someString: String
overrideableStringType: String
overridableListing1: Listing<String | Int>
overridableListing2: Listing<String | Int>
overridableMap1: Map<String, String | Int>
overridableMap2: Map<String, String | Int>
overridableUnion1: Int|String|List<String>
overridableUnion2: Int|String
}

class MyConcreteClass extends MyAbstractClass {
anotherString: String
overrideableStringType: "string literal type"
overridableListing1: Listing<Int | String>
overridableListing2: Listing<String>
overridableMap1: Map<String, Int | String>
overridableMap2: Map<String, String>
overridableUnion1: String|Int
overridableUnion2: String
}
24 changes: 24 additions & 0 deletions codegen/snippet-tests/output/04_with_class.pkl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,37 @@ export interface MyAbstractClass {
someString: string

overrideableStringType: string

overridableListing1: Array<string | number>

overridableListing2: Array<string | number>

overridableMap1: Map<string, string | number>

overridableMap2: Map<string, string | number>

overridableUnion1: number | string | Array<string>

overridableUnion2: number | string
}

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

overrideableStringType: "string literal type"

overridableListing1: Array<number | string>

overridableListing2: Array<string>

overridableMap1: Map<string, number | string>

overridableMap2: Map<string, string>

overridableUnion1: string | number

overridableUnion2: string
}

// LoadFromPath loads the pkl module at the given path and evaluates it into a N04WithClass
Expand Down
49 changes: 43 additions & 6 deletions codegen/src/internal/ClassGen.pkl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ local function getAllProperties(clazz: reflect.Class?): List<reflect.Property> =

local function isSameType(typeA: reflect.Type, typeB: reflect.Type) =
if (typeA is reflect.DeclaredType && typeB is reflect.DeclaredType)
typeA.referent.reflectee == typeB.referent.reflectee
typeA.referent.reflectee == typeB.referent.reflectee &&
typeA.typeArguments.length == typeB.typeArguments.length &&
typeA.typeArguments
.zip(typeB.typeArguments)
.every((pair) -> isSameType(pair.first, pair.second))
else if (typeA is reflect.NullableType && typeB is reflect.NullableType)
isSameType(typeA.member, typeB.member)
else if (typeA is reflect.NothingType && typeB is reflect.NothingType)
Expand All @@ -57,8 +61,8 @@ local function isSameType(typeA: reflect.Type, typeB: reflect.Type) =
typeA.value == typeB.value
else if (typeA is reflect.UnionType && typeB is reflect.UnionType)
typeA.members.length == typeB.members.length &&
typeA.members
.zip(typeB.members)
typeA.members.sortBy((m) -> m.referent.reflectee.toString())
.zip(typeB.members.sortBy((m) -> m.referent.reflectee.toString()))
.every((pair) -> isSameType(pair.first, pair.second))
// remaining types: `FunctionType`, `TypeParameter`, `ModuleType`.
// we can actually check if `ModuleType` refers to the same type by checking if the enclosing declaration is the same,
Expand All @@ -70,9 +74,42 @@ local function isSameType(typeA: reflect.Type, typeB: reflect.Type) =
// 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
(
parentType is reflect.DeclaredType &&
(
(
// String type can be overridden by string literal type
parentType == reflect.stringType &&
childType is reflect.StringLiteralType
) ||
(
// Same type, different but compatible type arguments
childType is reflect.DeclaredType &&
parentType.referent.reflectee == childType.referent.reflectee &&
(
parentType.typeArguments
.zip(childType.typeArguments)
.every((pair) ->
isSameType(pair.first, pair.second) ||
isCompatibleType(pair.first, pair.second)
)
)
)
)
)
||
(
parentType is reflect.UnionType &&
(
(
// Child union can be a subset of the parent union's members
childType is reflect.UnionType &&
childType.members.every((m) -> parentType.members.contains(m))
)
// Or child type can be one of the types from the parent union
|| parentType.members.contains(childType)
)
)

// visible for testing
function getFields(
Expand Down
Loading