react-jsonschema-form
react-jsonschema-form copied to clipboard
rjsf-team
Omit extra data in schemaUtils
Reasons for making this change
fixes #4081
For now I implemented the draft version (code and markdown documentation is not finished yet). Would you be able to look if everything seems alright with the progress so far?
Checklist
- [x] I'm updating documentation
- [x] I've checked the rendering of the Markdown text I've added
- [x] I'm adding or updating code
- [ ] I'm adding a new feature
- [ ] I've updated the playground with an example use of the feature
@MarekBodingerBA There is one failing tests, and you also have to get the test coverage back up to 100% given the new code you wrote
@MarekBodingerBA There is one failing tests, and you also have to get the test coverage back up to 100% given the new code you wrote
Of course, I am still working on it, it's a draft afterall . :)
@MarekBodingerBA There is one failing tests, and you also have to get the test coverage back up to 100% given the new code you wrote
Of course, I am still working on it, it's a draft afterall . :)
Awesome! how do you feel it is going? Are you needing any help?
@MarekBodingerBA There is one failing tests, and you also have to get the test coverage back up to 100% given the new code you wrote
Of course, I am still working on it, it's a draft afterall . :)
Awesome! how do you feel it is going? Are you needing any help?
Actually, the failing test is a bug in the original code that I've only migrated and created a test that found this.
I would probably like to disable the test and create a separate issue for this (as there would be no regression). The thing is, that I find getFieldNames implementation a bit overblown (and buggy in this case) for what it does, but I think it is better do to in a separate issue and don't mix those two together.
@MarekBodingerBA There is one failing tests, and you also have to get the test coverage back up to 100% given the new code you wrote
Of course, I am still working on it, it's a draft afterall . :)
Awesome! how do you feel it is going? Are you needing any help?
Actually, the failing test is a bug in the original code that I've only migrated and created a test that found this.
I would probably like to disable the test and create a separate issue for this (as there would be no regression). The thing is, that I find
getFieldNamesimplementation a bit overblown (and buggy in this case) for what it does, but I think it is better do to in a separate issue and don't mix those two together.
That is fine, as long as we get that bug fixed soon after we merge this
@MarekBodingerBA How is this work going? There seem to be several new issues related to this feature and I would love to get this part refactored
@MarekBodinger FYI, I just merged a PR that will definitely force you to deal with the conflicts. How can I support you in getting this code merged?
Hi @heath-freenome,
sorry for not responding. I've got stuck with this pull request.
As far as I remember, it seems that in some cases the schema must be retrieved (as it always is in Form component where the original code) is. When I migrated the standalone implementation it started to fail for pretty ordinary schemas (as you can see in the test case). Then I started to digging into getFieldNames and I realized the best would be to completely rewrite the function, but wasn't really sure how to proceed with this safely.
Merge the code please, I am curious what's new. I would like to proceed in future with this, but don't have a capacity to do so now.
@MarekBodingerBA I can't quite merge the code as there are conflicts. Would you be willing to resolve them and comment out the one failing test. Then we can write an issue to deal with fixing the test and the bug associated with it. Also, a change was recently made to Form which is probably the source of the conflicts and you may need to fiddle with the code some to resolve them correctly.