kettanaito / react-advanced-form

Functional reactive forms. Multi-layer validation, custom styling, field grouping, reactive props, and much more.

https://redd.gitbook.io/react-advanced-form

MIT License

217 stars 24 forks source link

Cannot read "props" of undefined (formless field) #357

Closed kettanaito closed 5 years ago

kettanaito commented 5 years ago

Environment

react-advanaced-form: 1.6.4

What

Current behavior

When rendering a field without a parent Form, it will throw the mentioned exception in the console.

Expected behavior

Ideally, #216, yet for now need to add a better explanation of what the error is about.

Whenever a field is created without a Form passed to it through the context, a meaningful error must be displayed in the console
The rendering of the page must not be blocked; formless field returns null from its render method.

Why

Need to have good DX, errors must be transparent.

How

Check this.context.form upon Field mount.
If undefined, throw a verbose error, prevent from attempting to register.

redraushan commented 5 years ago

Hi @kettanaito,

Here latest release tag shows 1.6.0 however, in npm its 1.6.6, does that mean github code is not in sync with npm packages?

kettanaito commented 5 years ago

Hello, @redraushan.

GitHub code and the published versions are always up-to-date, I simply don't create new release notes for a patch update. I'm not sure if this is the common practice, but I provide patch updates without a dedicated GitHub release, by bumping a patch version and releasing the patched package to npm. Then the next minor version lists all the bugfixes (patches) provided in-between two minor versions in its release notes.

Take a look at 1.6.0 Release notes and its "Bugfixes" section. Those are patches provided between versions 1.5.0 and 1.6.0. Since the newest patch version is usually installed by default, it's safe to update to the latest patch version to have all the found bugs fixed.

Please, does this answer your question?

redraushan commented 5 years ago

Hi @kettanaito,

Aha, now I got it. Thanks for elaborating in detail, it definitely makes sense not to release paches/bugfixes. I am kind of new into OSS and contributions, looking forward to learn from all of you :) Is this repo associated with any gitter or other place for communication ?

Thanks

kettanaito commented 5 years ago

No worries. Glad it cleared it out for you.

There is a React Advanced Form Discord channel for communication. You are welcome to stop by.

kettanaito commented 5 years ago

Actually, I think I've covered this story here. Please, if you decide to contribute, could you double check first if it warns you when rendering a field without a parent form? Thanks.

redraushan commented 5 years ago

Sure, let me confirm the same if it works then we cam move to other issues after closing it.

redraushan commented 5 years ago

Hi @kettanaito,

I have just confirmed the behaviour, it throws constructive warning if Fields are not wrapped around Form, however it also breaks the rendering. Is it expected to be broken unless wrapped around Form?

If yes then I think we can close the issue. Thanks

kettanaito commented 5 years ago

Thanks for checking, @redraushan.

That screenshot, however, doesn't include the error I've added. I wonder why. Could you please double check the rest of the errors in the browser's console?

We shouldn't break the rendering, you are right. I think if we analyze the presence of the parent form passed through context (in the createField.constructor), we can omit the field's registration, and render null in its render method, so the rendering is not blocked.

I am usually available in the mentioned discord channel, so feel free to reach out for the questions. I would be glad to help. Thank you!