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 guards for potentially undefined field #22

Merged

Conversation

hanna-greaves
Copy link
Contributor

It is possible that the validations on an entity field can be undefined, leading to type errors. This change adds guards to avoid this situation.

Resolves #18

@marcolink
Copy link
Collaborator

@hanna-greaves awesome - thank you so much 🎉

I'm wondering if we should add some tests for it, wdyt?

@hanna-greaves
Copy link
Contributor Author

Ah yeah probably a good idea! :) I will take a look at adding some asap!

@hanna-greaves
Copy link
Contributor Author

@marcolink I've added a render test, I'd like to add a definition test as well, but the Definitely Typed definitions need to be updated to make FieldValidations optional, since they're not guaranteed to be defined.

Would you be okay with merging this change and then me creating another PR to add a test for that once I've made an update for the Definitely Typed definition?

@marcolink
Copy link
Collaborator

@hanna-greaves looks great, will merge it right away!

@marcolink marcolink merged commit fb4dcec into contentful-userland:master Sep 9, 2020
@github-actions
Copy link

github-actions bot commented Sep 9, 2020

🎉 This PR is included in version 1.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@hanna-greaves hanna-greaves deleted the validations-guard branch September 9, 2020 15:36
@marcolink
Copy link
Collaborator

@hanna-greaves and regarding your question about types in need of an update,
What exact package would that be?

@hanna-greaves
Copy link
Contributor Author

I had thought the types were in definitely typed, but they're coming from the main Contentful package :) I am writing up an issue now if you don't mind me @'ing you on it?

This line specifically is the problem

@marcolink
Copy link
Collaborator

Ah, interesting. And yes, feel free to add me :)

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

Successfully merging this pull request may close these issues.

🐛 TypeError: Cannot read property 'length' of undefined
2 participants