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 type-safety to Record class #532

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

parzhitsky
Copy link
Contributor

Description

This PR adds ability to create type-safe Record entities by assigning type to Record's value.
The implementation is based on adding type parameters to types, interfaces, and classes. It is implemented however in a backwards-compatible manner, so that opting in to type safety is optional.


Usage

const record = new Record<Person>(['name', 'age'], ['Alice', 20])
Before (unsafe):
record.get('neam') // typo, but no errors

record.toObject().neam[0] // run-time (late) error
After (safe):
record.get('neam')
// Error: property 'neam' does not exist

record.toObject().neam[0] // design-time (early) error

image

@parzhitsky parzhitsky mentioned this pull request Mar 5, 2020
@martin-neotech
Copy link
Contributor

Hi @parzhitsky can you please resolve the merge conflicts, thanks for contributing.

Cheers Martin

@parzhitsky parzhitsky force-pushed the feature/record-extended-types branch from c8e7409 to 437b430 Compare March 12, 2020 13:29
@parzhitsky
Copy link
Contributor Author

Is there anything else I can do with this PR?

@martin-neotech
Copy link
Contributor

@parzhitsky Sorry for the delay, we do not have any one at the moment to review your Typescript improvements.

@parzhitsky
Copy link
Contributor Author

parzhitsky commented Apr 3, 2020

Oh, got it. Please take your time, no worries

@leefreemanxyz
Copy link

Is there any chance of this getting merged soon? It would be a really nice feature to have.

test/types/record.test.ts Outdated Show resolved Hide resolved
@bigmontz
Copy link
Contributor

Sorry for the delay, @parzhitsky. Could you please move the target branch to 4.3?

@parzhitsky parzhitsky marked this pull request as draft November 30, 2020 10:02
@parzhitsky parzhitsky changed the base branch from 4.0 to 4.3 November 30, 2020 10:02
@parzhitsky parzhitsky marked this pull request as ready for review November 30, 2020 10:10
@parzhitsky parzhitsky marked this pull request as draft December 4, 2020 10:34
This commit adds ability to create type-safe Record entities by assigning type to Record's value.
The implementation is based on adding type parameters to types, interfaces, and classes. It is implemented however in a backwards-compatible manner, so that opting in to type safety is optional.

Examples:

// before (unsafe)
const record = new Record(['name'], ['Alice'])
record.get('neam') // no errors
record.toObject().neam[0] // run-time (late) error

// after (safe)
const record = new Record<Person>(['name'], ['Alice'])
record.get('neam') // Error: does not exist
record.toObject().neam[0] // design-time (early) error
@parzhitsky parzhitsky marked this pull request as ready for review December 4, 2020 16:07
Copy link
Contributor

@bigmontz bigmontz left a comment

Choose a reason for hiding this comment

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

@parzhitsky Thanks for your contribution, should i merge it?

I will back port to 4.2 after merged, but I will wait a bit for the release it.

@parzhitsky
Copy link
Contributor Author

@bigmontz Sorry, I didn’t get the point. Could you rephrase it? No offense 🙂

Should be okay to merge the PR.

@bigmontz
Copy link
Contributor

bigmontz commented Dec 7, 2020

@bigmontz Sorry, I didn’t get the point. Could you rephrase it? No offense 🙂

Should be okay to merge the PR.

I will release this feature in the 4.2.x in the near future (one week, or something like this), I will wait a bit to see if I manage to join it with other improvements or fixes.

@bigmontz bigmontz merged commit d0b66c3 into neo4j:4.3 Dec 7, 2020
@bigmontz
Copy link
Contributor

Hi @parzhitsky, I've released this improvement on the version 4.2.2 of the driver.

Thanks for your contribution.

@parzhitsky
Copy link
Contributor Author

Happy to help! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants