Skip to content

Make validate support optional fields#562

Closed
mkmik wants to merge 1 commit intoprotobufjs:masterfrom
mkmik:master
Closed

Make validate support optional fields#562
mkmik wants to merge 1 commit intoprotobufjs:masterfrom
mkmik:master

Conversation

@mkmik
Copy link
Copy Markdown
Contributor

@mkmik mkmik commented Dec 15, 2016

No description provided.

@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Dec 15, 2016

Optional fields should already be checked here when present.

Are you experiencing any issues? Maybe null should be checked there, too.

dcodeIO added a commit that referenced this pull request Dec 15, 2016
@mkmik
Copy link
Copy Markdown
Contributor Author

mkmik commented Dec 15, 2016

yes I added this check because that generated function was entered with m == null when an optional field was omitted. or at least the stack trace said that null has no such property. afk atm, I can attach a failing case when I'm back

@dcodeIO
Copy link
Copy Markdown
Member

dcodeIO commented Dec 15, 2016

I assume that the null value there is representing an optional message. I added a commit above, give it a try when you're back.

@mkmik
Copy link
Copy Markdown
Contributor Author

mkmik commented Dec 16, 2016

I confirm that master no longer exhibits the issue, abandoning this PR. Thank you!

@mkmik mkmik closed this Dec 16, 2016
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.

2 participants