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

feat: adds raw property to error and debug parameters #167

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

stalniy
Copy link
Contributor

@stalniy stalniy commented Mar 29, 2021

Fixes #140

@stalniy
Copy link
Contributor Author

stalniy commented Mar 29, 2021

There is a breaking change in Parameter<T> type but this is actually not a breaking change but a fix. Because Parameter<number> currently is equivalent of { type: number, value: number } but it's not TRUE in runtime because Parameter<number>["value"] is always string (it's a serializable value).

@Minigugus please correct me if I missed something

@stalniy
Copy link
Contributor Author

stalniy commented Mar 29, 2021

Also I'd like to get a new beta release when this is merged :) @porsager could you please clarify when you plan to release it?

@porsager
Copy link
Owner

porsager commented Mar 29, 2021 via email

Copy link
Contributor

@Minigugus Minigugus left a comment

Choose a reason for hiding this comment

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

@stalniy I just realized that value is the result of the serialize method from custom types, so you're right, it's always string, my bad 😕

Your fix looks good to me 👍

@stalniy
Copy link
Contributor Author

stalniy commented Mar 29, 2021

by the way, it's not a string in all cases. there is a possibility for it to be null. So, probably better to change it to string | null, what are you thoughts guys?

@porsager
Copy link
Owner

porsager commented Apr 2, 2021

Sorry, vacation with children is unpredictable. I'm first at the computer now. Do you want to have the string | null part included in this PR before I merge?

@stalniy
Copy link
Contributor Author

stalniy commented Apr 3, 2021

Yes. I think it should even be string | undefined | null. Because there is non strict equality but I need to test it first. And ensure that this are actually a valid cases in runtime.

@stalniy
Copy link
Contributor Author

stalniy commented Apr 5, 2021

undefined is not possible in runtime. The library throws an error before running query. That means neither debug not query with parameters property are created. So, it's safe to type it as string | null

@stalniy
Copy link
Contributor Author

stalniy commented Apr 5, 2021

updated

@porsager
Copy link
Owner

porsager commented Apr 5, 2021

Exactly 😊

Looks good. I'll release a new beta tonight

@porsager porsager merged commit 9e83e7f into porsager:master Apr 5, 2021
@stalniy stalniy deleted the feat/raw-params branch April 5, 2021 08:55
@stalniy
Copy link
Contributor Author

stalniy commented Apr 12, 2021

@porsager is there anything I can do to make beta.5 released? I would like to use those new features in my apps :)

@porsager
Copy link
Owner

🙈 Yeah, your reminder just now should do it - sorry I forgot. I'll get it out now.

@porsager
Copy link
Owner

porsager commented Apr 12, 2021

Released 😉

@stalniy
Copy link
Contributor Author

stalniy commented Apr 12, 2021

Awesome! Thank you!

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.

Expose raw values for parameters, before serialization
3 participants