-
Notifications
You must be signed in to change notification settings - Fork 535
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
feat: customizeSchemaTyping #23084
base: main
Are you sure you want to change the base?
feat: customizeSchemaTyping #23084
Conversation
Co-authored-by: Joshua Smithrud <[email protected]>
"section": tree | ||
--- | ||
|
||
Disallow some invalid and unsafe ObjectNode field assignments at compile time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR builds upon #23053 . This changeset will probably need to be rewritten based on the major changes.
|
||
it("customized narrowing", () => { | ||
class Specific extends schema.object("Specific", { | ||
s: customizeSchemaTyping(schema.string).simplified<"foo" | "bar">(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shows customizeSchemaTyping being used to implement a previously requested feature of narrowing leaf types.
It can also be used for branding of lead types, which would be even safer from a soundness perspective.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks cool
it("mutable static registry, safer editing API", () => { | ||
const ItemTypes: ItemSchema[] = []; | ||
class Container extends sf.object("Container", { | ||
child: sf.optional(customizeSchemaTyping(ItemTypes).simplified<Item>()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file shows several ways to do open polymorphism / schema child dependency injection. This one uses customizeSchemaTyping, and makes everything just work.
container.child = TextItem.default(); | ||
container.child = undefined; | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to try something like this:
interface A {
type: "a";
foo: string;
}
interface B {
type: "b";
bar: number;
}
interface C {
easy: number
}
type Combo = A | B;
interface Root {
faz: Combo;
c: C;
}
class SyncedRoot extends sf.object(...) implements Root {}
...
const a: A = { ... }
const root: Root = {...}
const syncedRoot = new SyncedRoot(root);
syncedRoot.faz = new SyncedA(a);
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Coverage Summary
↓ packages.dds.tree.src.simple-tree.api:
Line Coverage Change: -0.05% Branch Coverage Change: No change
Metric Name | Baseline coverage | PR coverage | Coverage Diff |
---|---|---|---|
Branch Coverage | 88.76% | 88.76% | → No change |
Line Coverage | 82.35% | 82.30% | ↓ -0.05% |
↑ packages.dds.tree.src.simple-tree:
Line Coverage Change: 0.15% Branch Coverage Change: 0.02%
Metric Name | Baseline coverage | PR coverage | Coverage Diff |
---|---|---|---|
Branch Coverage | 94.07% | 94.09% | ↑ 0.02% |
Line Coverage | 97.23% | 97.38% | ↑ 0.15% |
Baseline commit: e603044
Baseline build: 307393
Happy Coding!!
Description
Add
customizeSchemaTyping
.Status: Proof of concept / Proposal.
Before this could be mergeed it would need:
Breaking Changes
TODO
Reviewer Guidance
The review process is outlined on this wiki page.