Remove react-jsonschema-form dependency

[annekainicUSDS]

A few months ago (~March, 2018), the Vets.gov team decided to fork react-jsonschema-form, and the forms library has been using this forked version ever since.

The reasons we made this change at the time were:

1. The version immediately after the one we’re using (v0.43), they made a breaking change to how array fields are treated, which doesn’t fit with how we use them. It’s not likely that we could get them to switch back, I don’t think.
2. The above blocks us upgrading to React 16 for the forms code, because the updates for that are after the above change.
3. In the latest version, they switched JSON Schema validator libraries. The new one they’re using (ajv) looks much larger than the current one and isn’t something we would want force users to download.

At the time we submitted a ticket to the react-jsonschema-form library asking about the dependency size, but haven't received an answer yet.

Revisit if these reasons are still valid, and if we can find a way to move back to using the original library.

[dmethvin-gov]

So is this the change that caused trouble with arrays? It is prefilling an array with defaults if you define a min size. rjsf-team/react-jsonschema-form#484

How much bigger was ajv versus json-validator? The sample app is already too big so I agree it could be an issue, but maybe there are some other ways to reduce size.

I feel like we're playing with fire by not updating this dependency at all. If we could live with ajv's size we might be able to keep our fork updated since it's only a few lines of code for that change.

[annekainicUSDS]

Looks like other users had problems with this change as well: rjsf-team/react-jsonschema-form#434.

It was subsequently refactored (rjsf-team/react-jsonschema-form#534); the code now looks like: https://github.com/mozilla-services/react-jsonschema-form/blob/master/src/utils.js#L151. Need to investigate if this change fits with how we handle arrays.

[annekainicUSDS]

Nevermind, that wasn't the change that created a problem for us. We submitted a PR to rjsf to fix the issue for us, but it hasn't been merged yet: rjsf-team/react-jsonschema-form#871.

[jbalboni]

Here's the doc I wrote explaining the reasoning behind the fork: https://github.com/department-of-veterans-affairs/vets.gov-team/blob/18b28c815e1792fa21041cb1ea0f0e8539a6ad7f/Work%20Practices/Engineering/DocumentedDecisions/RJSF_fork.md

I very, very strongly think you should not attempt to go back to the main version of RJSF. I think the better path forward to is remove the unused code in RJSF fork repo and move the remaining code into the us-forms-system repo. There's very little code in RJSF that is actually used by us-forms-system. It's largely the top level Form component and the utils code.

The minified version of ajv is 119kb. I don't remember exactly, but jsonschema was something like 20kb or 30kb minified (the unminified code is 50kb). It's larger than react-dom, which I think is the largest dependency our form applications have. The us-forms-system library is already very large, and adopting ajv puts it over what I would consider reasonable.

Something I was thinking about was trying to make the validation pluggable, so that people could use different validation engines. Errors are already passed into the components in a set format that could be tweaked to be more generic. That would let us-forms-system pick a reasonable validator like jsonschema, let someone with other requirements choose ajv, and let someone trying to reduce bundle size implement a custom validator that doesn't do a full JSON Schema implementation.

[dmethvin-gov]

@jbalboni thanks! I'm still coming up to speed and it's really helpful to know the path that got us here. The vets.gov-team repo is private so I can't see that at the moment; if you don't mind giving me access you could do that or post the info elsewhere. @annekainicUSDS has filled me in on a lot of it already but every time I think I know what's going on I find more to wonder about.

The idea of just pulling in the part we need sounds interesting and would give us a chance to revisit any design decisions that are causing us pain. RJSF is Apache-2.0 licensed so that wouldn't be a problem. It's always a worry when there are 200 issues and 50 un-landed pull requests in a repo that your own project depends on.

[jbalboni]

Yeah, when we first started using RJSF they seemed to be more actively maintaining the library. I submitted a few PRs and got them merged. But a PR I submitted recently has been sitting there for months.

I can't grant access to that repo, but I'll paste the content below since it's not sensitive in any way:

Moving to a fork of RJSF

Decision Made:	Yes	Decision Date:	03/2018
Revisit Decision:	Yes	Revisit Date:	July 2018

Revisit Criteria: As of 3/2018, we're just starting our fork. After using it for a couple months, we should revist and make sure it's still the best path.

Decision Makers: @annekainicUSDS @jbalboni

---

tl;dr

We've diverged from the react-jsonschema-form functionality enough that forking the library is the best way to continue improving our form building process.

History

We've been using a set of components built on top of RJSF for all of our longer forms. This has been working pretty well, but in the process of upgrading RJSF to work with a newer version of React, we've run into several issues. These issues, coupled with the fact that we use very little of the RJSF code currently, suggest that forking is the better option

Breaking or suboptimal changes in the latest RJSF version

• Switched JSON Schema validators from jsonschema to ajv. The ajv validator is faster and has more detailed error output, but is also much larger. It nearly doubles the size of RJSF. We have not run into any major issues with jsonschema and one of our goals is to reduce the bundle size of our forms.
• Moved defaulting logic out of widgets. This means that all of our widgets now get form data that has empty records, which is contrary to how they were written. It makes it harder to determine when a field is actually empty vs. just defaulted.
• Maintains internal state for form data. This is not new, but it causes bugs in our application with autofill, because rapid onChange events can cause stale data to overwrite the current form info. This is a design choice, so it's not really a bug to fix in the library.
• There's a bug in how the default logic for arrays with minItems works. This is fixable in the library and we've submitted a PR, but since we can't update to the latest version due to the above issues, we can't pull it in.
• Similarly, we need the changes that have been made to support React 16, but those are post some of the above changes.

Pros

• We have more control over the code for our forms, making it easier to fix bugs
• Forking addresses all of the above issues
• We can remove widget and field code that we don't use, making it easier for us to maintain. Eventually that may mean getting away from a separate library entirely.

Cons

• We take on extra maintenance burden, since this is not code we initially developed. This could include things like security issues.

Example of an alternative approach

We could attempt to move to the latest version and push for the above changes in the main repo, but it's not guaranteed that we could be successful, or even that all of our changes make sense for the main library.

Decision

We're going to fork RJSF.

[annekainicUSDS]

Things that we directly import from react-jsonschema-form right now:

1. <Form /> from here
2. deepEquals from here
3. toIdSchema from here
4. getDefaultFormState from here
5. orderProperties from here
6. getDefaultRegistry from here
7. shouldRender from here
8. getWidget from here
9. getUiOptions from here
10. optionsList from here
11. asNumber from here

Things we indirectly use because they're called by things we directly call

1. <SchemaField />
2. every field in components/fields/ that isn't already in us-forms-system
3. every widget in components/widgets/ that isn't already in us-forms-system

^^ still working on completing this list

[jcmeloni-usds]

@annekainicUSDS @dmethvin-gov What is the current state of this question, in y'all's minds?

[dmethvin-gov]

I think we should drop RJSF as a dependency and find a better solution. We're not using that much of it and the notation it's imposed on the library (the schema vs uiSchema dichotomy, ephemeral view: fields) makes it hard to define forms. This is part of the phase 2 discussion.

[annekainicUSDS]

I think Dave is hinting at 2 separate concerns that may have different solutions:

We're not using that much of it

Something we've discussed is pulling the few things we are using from RJSF into USFS so we can remove it as a dependency. This makes us wholly responsible for these parts of the code, but it's not like we were getting a ton of support from the maintainers of RJSF for changes we requested anyway.

the notation it's imposed on the library makes it hard to define forms

This would be a much broader refactor and departure from RJSF, as opposed to just copying in some files as described above.

[dmethvin-gov]

I agree, these are separate concerns.

[jcmeloni-usds]

Got it. I'm up to speed now. Thanks!

[epicfaace]

Hi, @dmethvin-gov, @annekainicUSDS , @jbalboni ,

I recently joined react-jsonschema-form as a maintainer and I'd be willing to work with you on seeing if there is a way to make the react-jsonschema-form library work for you. As you know, that library hasn't been too active for a while, but we're aiming to revive development and be more responsive to issues and pull requests.

We've already merged in rjsf-team/react-jsonschema-form/pull/871; thanks for that change. If you're on board, I can work with you to make sure fixes to your other concerns are incorporated into the library upstream.

It's my strong belief that allowing RJSF and USFS to work together will benefit both projects. Improving RJSF to be able to handle your requirements will help make it more versatile, in addition to the obvious benefits in collaboration two open source projects have when sharing a common code base.

[dmethvin-gov]

Hi @epicfaace, sorry about the long delay. At the moment we've stepped back to think about the design of the library and the best way to expose the features. There's no set timeline for the work.

The current RJSF design results in information about a field being spread across several different places. The VA created some tools to help with building the schema, but then those have to be pulled apart on each form and put into its place. There just seems to be a lot of boilerplate and data shuffling going on.

This might be the spec on a typical field, say a social security number:

• The field label is "Social Security Number"
• It's a string (don't treat it as a number with an up/down spinner)
• If possible give the user a numeric keypad on mobile to simplify input
• Use CSS style us-user-ssn on the input
• The user can enter - or a space but we'll strip those out when focus leaves
• If it's not 10 digits (0-9) at that point give an error to the user
• This field is required
• We want to send a 10-digit string to the server
• The field name is user-ssn

Only the last three items apply to the JSON Schema, the other info is used only by the client. If more information needs to be added it's also likely to be about the client formatting or behavior (e.g., additional CSS classes). The same field name might need to be mentioned four times during definition (schema, uiSchema, initialValue, and required) which is verbose and subject to typos.

With that in mind, I'm thinking it would help to define a more ergonomic data structure from which the JSON Schema could be created. Is the project in maintenance mode, or would it make sense to collaborate on creating something new based on the concept of building a client-side form using a JSON-based definition that is a superset of JSON Schema?

[epicfaace]

@dmethvin-gov , I'd think that all the features on the spec for the SSN field could theoretically be implemented by a uiSchema. Perhaps one way of going about this would be to implement the proposal to combine the uiSchema and schema into a single JSON schema (see rjsf-team/react-jsonschema-form#701). I'm open to big changes in react-jsonschema-form if they are warranted, but ideally I'd like to keep the library keep to the spirit of generating a form from a JSON Schema.

I'd like to understand what exactly you mean, though. Do you think a uiSchema would be enough to implement the spec for a SSN field that you mentioned? What do you mean by initialValue? And why is it a problem to have to mention field names multiple times in the required array in the JSON Schema (what alternatives would you have in mind for this point)?

[dmethvin-gov]

Do you think a uiSchema would be enough to implement the spec for a SSN field that you mentioned?

Yes, it's all doable today but not very pretty. See below.

What do you mean by initialValue?

Sorry, I meant initialData. VA forms need a way to set an initial value when the forms load in order to implement form save/restore functionality for long forms where the user may come back later to finish.

And why is it a problem to have to mention field names multiple times in the required array in the JSON Schema (what alternatives would you have in mind for this point)?

It wasn't that mentioning a field name in required is a problem, it's that the name may need to be repeated several times in a definition.

Anyway, the extensions that the VA team put on top of RJSF are an example of the things that are missing, particularly with multi-page forms. The VA forms are a good place to start as far as what features or changes might help--who knows, maybe there is a better way to do it using RJSF. Within the current limits of the current schema/uiSchema dichotomy though, I think the VA forms do about as good a job as they could.

Here are some of the things I've found while looking at the problem of large forms (both large in the number of fields in a form and large in the number of forms).

JSON Schema represents the back-end static data model, not the front-end dynamic UI: The two do not map 1:1 in all cases. In particular, the UI may want to apply some visual grouping that does not imply any sort of data nesting in the schema. USFS has view fields for that but it's basically a workaround for JSON Schema constraints. There are also cases where a form is not schema-valid and we remove user data immediately to get it to validate which can cause users to lose things they typed.

RJSF does not use a single-part JSON Schema: The schema has to be broken up into many pieces so that it can be interleaved with other information like the UI design and put into the form definition file. If the JSON Schema is going to be used in the back-end for validation (as it should be) then it has to be used as a whole. For small examples you could split or reassemble manually but for large numbers of fields and many forms that is definitely not advisable.

There is too much boilerplate and clutter: The best way to see this is to trace the information for one of the large VA forms.

• JSON Schema source: Looks pretty clean and is high level, but lacks the detail needed for rendering on the screen.
• Generated JSON Schema: Generated from the definition above. A bit more verbose than above but still logical. Could be cleaned up further with some more $refs.
• Form definition: Super verbose and hard to read. Pieces of fullSchema1990 pulled out and inserted into the form definition. Many helper functions and on-the-fly modifications to the formal schema due to data differences on one particular form. Lots of procedural code has to be used to generate this declarative spec, and a few procedural things still remain.

It just feels to me like this isn't encapsulating/abstracting enough information when you define a form, and as a result we end up with a lot of messy verbose stuff in our form definition. In the form definition proper I want to just say "This is a field named ssn of type SSN with a label Social Security Number and it's required" and everything else (validation, data types, styles, implementation, etc.) should be pulled in from the definition of SSN somewhere else so it can be reused without being re-chanted each time.

It would be great to get the perspective of the folks who have been working on this heavily like @jbalboni and @rianfowler as well.

[epicfaace]

Thanks for the writeup. I'm still not sure as to why VA created different schema and uiSchema objects for each page, though. This seems to have caused the necessity to break up the schema into many pieces, and led to, for example, "pieces of fullSchema1990 pulled out and inserted into the form definition."

Why not go with an approach that uses a single schema and the uiSchema itself defines the pages? Such as this proposal (in part, inspired by what I first saw on USFS): rjsf-team/react-jsonschema-form#1157

The proposal above is an extension of ui:order, but rather uses pages objects to allow for more granularity on how the fields are arranged. One could imagine that we could define a view property to each page object to solve the problem of grouping fields in the UI that should not be nested in the actual schema.

[jbalboni]

We went with the different schema/uiSchema per page approach because that was the only option available at the time (a couple years ago) that didn't require making changes to RJSF.

The proposal you linked might work for simple cases, but it would need a lot more functionality to cover all of our use cases. Our multiple page form config can skip pages based on form data and also works with react-router so that each form page has a matching url change. We also have different types of pages depending on the data in the form. For example, we have a way of specifying that the config should render a page for each item in an array, and different ways of ordering those pages.

I haven't thought too deeply about this, but I kinda feel like RJSF should avoid handling multiple pages. Once you start adding that you start taking on a lot more complexity and start defining things about how pages and page flow are handled, which expands what RJSF is responsible for. We made a lot of choices around how those should work for the VA forms and @dmethvin-gov and his team found that those choices aren't always the choices that other teams would make.

[dmethvin-gov]

I could see the benefit of letting RJSF just handle the one-page case, but if that's the goal it might be easier to pull in the parts we need and adapt them to make things more consistent. For example, we use strings to reference RJSF built-in components but React references for our own.

Once you get to multi-page forms the complexity definitely goes up, but in some ways that just means it's even more important to find ways to simplify things.

[dmethvin-gov]

For the current code base there are no plans to remove RJSF which would be a major bit of work. A future version may not include it but that would be part of a design overhaul so best addressed elsewhere.