puppetlabs / design-system

A resource for creating user interfaces based on brand, UX, and dev principles
https://puppet.style
Apache License 2.0
18 stars 27 forks source link

Using react-hot-loader breaks Form components #245

Open bradhe opened 4 years ago

bradhe commented 4 years ago

Describe the Bug

This is a pretty obscure bug that took me a bit to track down so bare with me on my description here. When using react-hot-loader (I'm currently using v4.12.21) users aren't able to type in to forms. The behavior is almost as if the keystrokes are getting eaten by an event handler: You type and nothing goes in to the form element. Internally, the state is never updated to indicate that the user typed anything in to the forms and so when you submit the form you just get empty values.

Based on my analysis of why the bug is occurring (I'll provide that below), this could actually be indicative of a bigger bug in the design system.

Expected Behavior

Using forms in either controller or uncontrolled modes should work A-OK.

Steps to Reproduce

All you need to do is create a new React project that uses react-hot-loader for hot module loading.

Environment

All?

Additional Context

So the underlying cause of the bug is a side effect of how the code is split/loaded by react-hot-loader and, additionally, how the Form component renders it's children. There are two render paths in the current implementation: One that is special-cased for FormField elements and one that is for all other children types.

export const renderChildren = (children, updatedFieldPropMap) =>
  React.Children.toArray(children)
    .filter(child => child)
    .map(child => {
      /**
       * If the child is a field, do special field rendering
       */
      if (componentHasType(child, FormField)) {
        return renderField(child, updatedFieldPropMap[child.props.name]);
      }

      /**
       * If the child has children, recurse. This will cover Form.Section and any wrapper divs
       */
      if (child.props && child.props.children) {
        return React.cloneElement(child, {
          children: renderChildren(child.props.children, updatedFieldPropMap),
        });
      }

      return child;
    });

The componentHasType function uses the React type metadata to identify components that are of type FormField here.

const componentHasType = (component, Type) =>
  component && component.type && component.type === Type;

component is of the correct type logically, but because the code is split by react-hot-loader the final check fails because the instance of FormField was instantiated by a different JavaScript file.

image

data for component.type

image

data for Type

This issue could be resolved by finding a different way of doing reflection, perhaps? I'll noodle on a patch.

bradhe commented 4 years ago

It looks like this is a known issue with react-hot-loader too...

link