WIP: Formatic 3000

[jdeal]

This is just the seed of a work in progress of a big potential project to rewrite Formatic. It could end up being scrapped, completed, or having bits of it worked into smaller PRs. For now though, I'm exercising a spec for a future version of Formatic by trying to implement it from scratch.

Tasks:

• Implement rough spec.
• Add hooks linting.
• ?
• Profit!

[stevemolitor]

I like this approach a lot - you can specify your fields with JSX and control where it goes, add wrapper stuff, etc.

I also really like the useField hook. I think that's a great way to write custom components or extend things.

Just my first impressions, but this is all feeling very nice!

[stevelikesmusic]

Echoing @stevemolitor, I'm definitely liking this. I'll try to go through building some components and checking out demo tomorrow. Code looks great 🙌

A few questions around style and syntax.

[stevelikesmusic]

Using the setupTestFrameworkScriptFile option in jest.config would call setup cleanup for all tests. Then we wouldn't need to explicitly call it in every test file.

[jdeal]

I guess that seems reasonable. The only downside of that is you can't opt out of that behavior then? For example, if you wanted to do some iterative tests on the same DOM, you could use afterAll, but with the global setting, your DOM would be wiped out each time.

[stevelikesmusic]

Super minor—another way to get this might be:

expect(
  getByLabelText('FIRST NAME').className.includes('css-')
).toBe(true);

[jdeal]

Agree, that's a little nicer!

[stevelikesmusic]

One question regarding syntax: does Formatic keep the same style guidelines we try to use company-wide? We say to always use arrow functions unless we need to keep context of this through the prototype chain, so as to document this's usage in a function.

I'm all for function ExampleForm, but just curious.

[jdeal]

It does use Zapier's linting, but I don't think it's stuck with all of our internal style rules.

This was just something I was trying though. I've found myself liking function more than arrow functions in a lot of cases. Using function makes it easier to export as a default for example.

[stevelikesmusic]

Totally agree

[stevelikesmusic]

Is the recommended pattern to wrap each form element in a div rather than:

<div>
  <label>
  <input>
</div>

// or 
<>
  <label>
  <input>
</>

[jdeal]

Hmm, good question. This is an example, so it doesn't matter here. But for built-in fields that might be exported, we should decide. I think built-in fields will probably mostly be for just getting something rendered quickly and probably won't be used in practice that much, so whatever renders the best out of the box?

[stevelikesmusic]

Do we really need this function if it only applies three fields to Tag (or whichever input)? I don't know if I see a lot of value for the abstraction over simply defining the fields.

[jdeal]

Probably not. The repeating _meta={meta} bugged me a bit and seemed like something that would easily get missed. And I didn't like the sort order rule of the linter changing from _tag="div" _key="Field" _meta={meta} to _key="div" _meta={meta} _tag="div". But yeah, it's not that big of deal:tm:.

[stevelikesmusic]

Is the intent for _component and other underscored props to imply do not mutate? Why wouldn't fieldType be _fieldType?

I've only really used _foo with object fields and methods—not as much with function args.

[jdeal]

The underscored props were just to signify that they would not be passed on to the component/tag. If a component had its own component prop, then you could still use it like:

<Component _component={MyComponent} component={PassThruComponent}/>

Also, the underscore pushes it the front in the alphabetized props, rather than have it arbitrarily mixed in.

[stevelikesmusic]

Do we need to put this in state? I could see caching the initial value of props.defaultValue.

[jdeal]

That's effectively what I'm doing here. When you use useState without changing the value, it's effectively like using a constructor in a class-based component. I'm saving the value for later rather than mutating it every time. The value passed to useState is the initial value and will never change here.

[stevelikesmusic]

Nice. Hooray for Context 🎉 Hooray for Hooks 🎉

[jdeal]

Glad you like it. 😄 This is actually the part I'm most nervous about, wondering if we should just keep Formatic out of the business of rendering components entirely. Where I'm at now: I think it's nice to have Formatic work out of the box, even if most consumers will tend to use hooks and provide their own presentational components. It's also good to have "reference components" to exercise the implementation.

It's really hard to come up with a good model for overriding the rendering of components though, and this is about as good as good as I think it gets. There's some extraneous wrapping here, but trying to avoid the wrapping starts making things a little weirder, like with the current version where you have to worry about ...children.

[stevenhauser]

@jdeal I took a very quick glance at this spec, but not the code. None of this seems problematic with how the design system is building components since largely this new version of formatic allows customizing how any form component renders 🎉

Overall this spec looks great to me. Here are some questions / feedback that I have:

1. There are a lot of examples and the API surface area still seems very large and somewhat ambiguous to me. Details below on individual points.
2. Is there a benefit of providing FieldContainer and hooks, as in does each solve unique problems that the other cannot? If they provide equivalent functionality then IMO there should only be one. If there's only one option then there's no choice to make, which I think is a good thing.
3. I don't follow the difference between renderTag and renderComponent. Does there need to be an actual difference between the two or could there only be one? I would expect only renderComponent to be necessary because I would think the rendered components would control their own DOM, so I don't follow where renderTag fits in and when it would be called.
4. I don't understand renderKey and am curious if it's necessary. I would assume that formatic would choose which component to render for me given the type of the field. How it compares to field.type is also confusing to me. If it is necessary, are there a finite number of values that renderKey could be? IMO it would make sense to provide those as constants rather than strings. Perhaps Symbols so you must import them and can't shoot yourself in the foot by having a lot of strings throughout the codebase.
5. Do nested values require nesting components or does/could fieldKey support something like my.nested.value or even ['my', 'nested', 'value']?
6. The AutoFields examples are a little hard for me to understand at a glance but seem to provide the functionality I would expect based on my above points. For instance mapping String: MyStringField and Integer: MyIntegerField makes a lot of sense to me. However I don't understand how AutoFields either complements or competes with renderTag and renderComponent or AutoFieldsContainer.

Overall it seems like there's a lot of API flexibility that is provided, but I think it makes it ambiguous which API to choose and how they compete with each other. If it comes down to personal preference rather than actual tradeoffs of one API vs another, I think formatic should choose one to minimize its API. I definitely think less is more here.

[jdeal]

@stevenhauser thanks for taking a look!

1. Hmm, I like to think the API surface is a lot simpler than the current version of Formatic, where the plugins have a lot of hidden surface area. But I'll answer specific points...
2. I see what you're saying, but I think that's a userland decision. Sometimes, it might make sense to inline a render callback because you have a bunch of variables in scope and you want to dynamically render a field based on those. Being forced to use useField would mean you have to pass all those variables as props in a case where it's not very ergonomic. Also, this makes it easy to make dependent fields where a field only shows up based on the value of another field. I'll likely create something higher level for that, but right now, I'm focusing on the primitives that you can stitch together. I appreciate wanting to keep the API small, and I know there's some overlap, but I think it's worth keeping the render callback for now. After I build more examples, I can revisit though.
3. I originally tried to do a single callback, but it gets pretty weird in practice. renderTag is specifically meant for rendering DOM nodes. Although it will probably be recommended that userland do all the DOM rendering, I think it's nice to have reference components to quickly build a form and see if Formatic works vs having to build custom components right away. renderTag exists mainly as a way to style the DOM elements of those reference components. renderComponent is used to completely take over the rendering of a component. If you take over all component rendering, you don't need to worry about renderTag at all.
4. I'm building the automatic form generation from other primitives. So you can have component/tag overriding independent of automatic form generation. I started out with symbols actually, but it felt kind of overkill. I'd be fine putting it back in though.
5. I imagine I'll add some sugar, but I wanted to focus on primitives first. I'd definitely want to avoid having to always parse the keys, so I'd want to make that a different prop like keyPath or something.
6. AutoFields mainly exists for you to map automatic form generation to your components. If you use built-in Formatic components, you'll be able to use renderComponent and renderTag on those, but if you use your own components, you would not (since you wouldn't have built your components to be overridable unless Formatic exports a utility for that).

Hopefully that helps explain some things @stevenhauser ! I kind of got lazy with the spec at the end, so as I flesh out more examples, it may make more sense. Or, maybe it will become more obvious what can be cut. Let me know if something I've said here still doesn't make sense though! (Happy to chat sync too.)

[jdeal]

Here's some more nuance worth knowing about renderComponent, renderTag, AutoFields, and AutoFieldsContainer:

renderComponent is meant for intercepting or overriding the rendering of components that would be rendered anyway if you did nothing. This means that you've already absorbed the cost of those components in the build, so you should either be intercepting to modify/add props or lightly overriding small components. You would not want to use renderComponent to completely replace a bunch of components, since you'd be including all those components in the build and then just replacing them anyway. This is a problem today with Formatic. All components are part of the default config, so even if you override the rendering of every single component, you're still paying the tax for all those default components.

That's one reason it's convenient to separate renderTag: with renderTag, you're rendering DOM nodes, so you want to render every single bit of the component. You just want to modify/add DOM-related props. Without that separation, you'd have to have a conditional inside your renderComponent function to determine what you're rendering. A data-* attribute makes sense in the case of a DOM-specific component, but not a DOM-independent component, for example.

AutoFields is a kitchen-sink component. It likely will exist simply to take Formatic for a test-drive or for consumers who don't care too much about presentation (like for admin pages) and just want to quickly dump a form to the page. It's going to absorb all of Formatic's built-in components into your build (like with today's Formatic). You don't want to use AutoFields and then override everything with renderComponent (although you totally could do that).

AutoFieldsContainer on the other hand takes a components prop so you can pass in your own components. Or you could pick and choose to import only those components you want from Formatic's built-in components vs importing the kitchen-sink with AutoFields. We can't have a single AutoFields component that does both these things, because once we've imported everything, we can't unimport everything. Tree-shaking has no idea which types you're going to pick at runtime.

Now, it does feel like the components prop of AutoFieldsContainer and renderComponent are doing similar jobs. But look at the signature of renderComponent:

renderComponent(renderKey, Component, props)

Right now, it's meant for overriding Component. But in the case of AutoFieldsContainer, we don't actually have a component to override. That's the point: we want to decide what to render in userland. If we did that, we'd be calling renderComponent with something like this:

renderComponent('Text', undefined, {...});

Maybe that's fine? It's a little weird though because it means that function is doing double-duty and like with the problem of combining renderTag, you'd have to have a conditional in there in some cases. And the documentation would have to explain that subtle/strange thing.

We could also have a third function like renderAutoFieldComponent or whatever, which would just be called like:

renderAutoFieldComponent('Text', {...});

That's clean and is a little more flexible than the components property since you can do some dynamic wrapping if you need to. However, that case is atypical and didn't seem to warrant the extra complexity vs just having an object pointing to components. If you really need to inject something dynamic, you can pretty easily do that with context and a hook. The reverse is true as well. You could use an object mapping instead of renderComponent, but there, we're talking about potentially many more layers of wrapping if you needed to do something dynamic, which might run into perf problems. (And there's already some wrapping cost to make the components overridable. I tried to avoid that, but that's another story of many traps.) So I tried to choose pragmatically for each use case.

Sorry about the wall of text! This is as much me thinking out loud as anything. So feel free to ignore. But happy to hear if you have any alternate design ideas.

[jdeal]

@stevelikesmusic I started implementing this, and it immediately feels unnecessary to do any kind of automatic coercion when you're specifying the field components anyway. It seems like the component's job to do this. In other words, better to just use an IntegerField component that does the proper coercion from text to integer if necessary. Formatic can provide a reference component for that, but even if it didn't, it would be trivial to build that from other primitives.

Once I get to the auto form generation which will require a schema, I still don't think it's really necessary: it just needs to pick the appropriate component based on the type to do the coercion vs doing any side-channel coercion.

If I get further and realize it is necessary for some reason, I'll pick this back up, but for now, I think I'm going to delete this section unless you can think of any reason to keep it?

Even if we do think we need it, I think this can be accomplished via a prop like shouldAutoCoerce or a special <AutoCoerceInput/>/<AutoCoerceField> component.

cc @stevenhauser

[jdeal]

@stevelikesmusic I made an even more performant ReactiveValue. (Unfortunately, it did get a bit more complex, but it's still nicely isolated behind hooks.) And as part of that, I added a useReactiveValueMeta that I swapped out here. It gives you access to property types and only updates when those types change. This is also dynamic, so new properties that are added will generate new auto fields.

I think this may be roughly how validation errors will work once we get to that. Effectively, validation errors will just be extra metadata this sits alongside the values.

[stevelikesmusic]

You can simplify this assert with expect(propertyTypesRenderSpy).toHaveBeenCalledTimes(1)