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

#118 use serialized key for poly switch #120

Merged
merged 10 commits into from
Feb 25, 2021

Conversation

ChrisDufourMB
Copy link
Collaborator

@ChrisDufourMB ChrisDufourMB commented Feb 8, 2021

Description

Note: This is a breaking change - @PolymorphicSwitch no longer supports static properties

Refactor the polymorphic switch decorator to use the same key serialization logic as the @SerializeProperty decorator.

@SerializeProperty()
@PolymorphicSwitch(() => new TestClass(), "TestClass")

Enhance the polymorphic switch decorator to accept a test function, that allows for custom logic when determining if the target property means that the polymorphic type is this class.

@PolymorphicSwitch(
  () => new TestClassOtherYear(),
  (propertyValue) => (propertyValue as Date).getFullYear() !== 2020,
)

Fixes #118

Renames input argument to json for polymorphic decorator functions

Things to look at

  • Test coverage
  • Code Style
  • Documentation (README.md, doc/, examples/, etc..)

Updates typescript types file
Rename "input" to "json" for polymorphic decorator functions
@ChrisDufourMB ChrisDufourMB added breaking changes Issues or fixes with breaking changes bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request labels Feb 8, 2021
Copy link
Contributor

@hardy925 hardy925 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey hey,

I am requesting changes because I ran the build script and our node tests failed.

Screen Shot 2021-02-11 at 5 22 30 PM

We are also missing two tests, if we cannot get a code path to those lines then we can probably make a small update

Screen Shot 2021-02-11 at 4 59 35 PM

Thanks!

Copy link
Contributor

@hardy925 hardy925 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me - 100% test coverage, all passing and our npm build looks good on the latest deno.

Thanks for the contribution

@ChrisDufourMB ChrisDufourMB merged commit 3025a65 into develop Feb 25, 2021
@ChrisDufourMB ChrisDufourMB deleted the #118-use-serialized-key-for-poly-switch branch February 25, 2021 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Issues or fixes with breaking changes bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Property key transforms are not applied when using @PolymorphicSwitch
2 participants