This repository has been archived by the owner on Jul 16, 2023. It is now read-only.
feat: base polycon code #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Chriscbr
changed the title
noyfactor
reviewed
Jul 26, 2022
noyfactor
reviewed
Jul 26, 2022
|
@Chriscbr this is great work. I feel like there are some gaps in the readme, and maybe a poc in the SDK repo could help highlight the APIs and their usage? |
Signed-off-by: github-actions <github-actions@github.com>
- Make sure `Polycon` extends `Construct` to ensure that languages that use nominal typing (such as Java, .NET, etc) will accept a polycon everywhere constructs are accepted (add a test). - Refactor the constructor path and use a marker on the scope to distinguish between polycon construction and the construction of the concrete type. - Add a few tests to verify some assumptions.
This was referenced Jul 28, 2022
This is a proposal based on [this comment](https://github.com/monadahq/polycons/pull/15/files#r933560022). The basic idea is that both the polycon class (e.g. `Dog`) and the concrete class (`Poodle`) extend a common abstract base type (e.g. `DogBase`). This has the following effects: 1. No need to protect against stack overflow (the concrete class doesn't actually extend the polycon). 2. It is now possible to instantiate any concrete type directly (added a test that instantiates a `Labrador` even if when `Dog` resolves to `Poodle`. 3. We can still share common implementation at the base. 4. Enforce that concrete classes actually implement all abstract methods and properties (previously this was only by convention). The polycon constructor has a quirky pattern: ```ts constructor(scope, id, props) { super(null as any, id, props); return Polycons.create(QUALIFIER, scope, id, props); } ``` The call to `super()` with `null` as the `scope` does two things: 1. The base construct is not attached to our tree. 2. The `null` can be used by the base class to bail out early: ```ts class DogBase { public readonly foo: number; constructor(scope, id, props) { super(scope, id); if (!scope) { // initialize all readonly props with dummy values, this obj is thrown away this.foo = -1; return; } this.foo = 7947; // <-- the real thing } } ```
eladb
reviewed
Aug 1, 2022
eladb
reviewed
Aug 2, 2022
eladb
reviewed
Aug 2, 2022
eladb
previously approved these changes
Aug 4, 2022
|
(added |
eladb
reviewed
Aug 4, 2022
| @@ -0,0 +1 @@ | |||
| TODO | |||
There was a problem hiding this comment.
@mbonig can you help with this?
eladb
approved these changes
Aug 4, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Setting up an initial version of the polycons API. Based on several iterations of the RFC, Mark's POC, my spinoff of Mark's POC, the team's code from our last hackathon, and many team members' feedback.
TODO:
Add contributing guidefuture