React-redux-form: Things in devDependencies that don't belong?

[ffxsam]

I was just sitting here cursing to myself as I launched an app into production, and react-redux-form totally failed to work. Typing in fields had no effect at all. Then I realized there's quite a long list of stuff in the devDependencies section of package.json, things like react, redux, etc. Do those really belong there? It seems to me that the following packages belong in dependencies instead: (or perhaps peerDependencies? I don't know what the difference is)

    "estraverse-fb": "^1.3.1", // not sure about this one..
    "immutable": "^3.7.6",
    "react": "^0.14.7",
    "react-addons-test-utils": "^0.14.7",
    "react-dom": "^0.14.7",
    "react-redux": "^4.4.0",
    "redux": "^3.3.1",
    "redux-thunk": "^2.0.1",

[davidkpiano]

Interesting. For sure, I know react, react-redux, and redux do belong in dependencies, and they are already there. I'll have to take a look at react-dom now - that could be the failure point - that's required for it to work as well.

What exactly was the failure?

[ffxsam]

I believe they belong in peerDependencies actually. That's how material-ui does it, at least.

The failure was that it simply wasn't working. Focusing in a field, typing in it.. none of those triggers Redux at all.

[davidkpiano]

Alright, I'll release a patch with redux-thunk and react-dom as peerDependencies right now. Can you let me know if that works in a couple minutes?

[ffxsam]

Absolutely!

[davidkpiano]

Okay, published as [email protected]. Try it out!

[ffxsam]

Trying it out now.. (PS, you'll want to remove the stuff in devDependencies that you copied over to peerDependencies.

[davidkpiano]

devDependencies won't get installed in production builds, so I think we're safe keeping them in there, especially when others are forking the repo. Otherwise, you'd have to manually install them.

[ffxsam]

The packages in devDependencies are covered by being in peerDependencies though. devDependencies is typically reserved for things like linting, testing, and dev/performance tools only.

[ffxsam]

Actually on second thought.. material-ui has react in devDependencies as well. I'm not sure why. But I'd just model yours after how they structured theirs :)

https://github.com/callemall/material-ui/blob/master/package.json

[davidkpiano]

It's because peerDependencies don't get installed automatically anymore - instead, NPM will just yell at you that they're missing. 😟

[ffxsam]

Ahhh ok. I'll admit, I don't totally understand npm's package system.

[ffxsam]

Man, I don't get it. It's still not working in production. Here are two screencaps.. first one is in production, the second is on my local dev machine.

[ffxsam]

But rrf is partially working in production. It can load data. Calls to it don't generate errors, but obviously fields aren't working.

[davidkpiano]

I have a feeling this has to do with material-ui itself, and not RRF. See this issue: https://github.com/davidkpiano/react-redux-form/issues/144

I'm going to investigate material-ui though. It makes things a lot more complicated, to say the least.

[ffxsam]

Oh shoot! Nice find. I've got my own field wrapper I'm using too, BTW:

export const MUITextField = createFieldClass({
  TextField: props => ({
    onChange: props.onChange,
    value: props.modelValue,
  }),
});

[davidkpiano]

Can you humor me and try doing it manually? Just to see if this works:

import { actions } from 'react-redux-form';


// ... in render
<Text Field onChange={(e) => dispatch(actions.onChange(e))} />

[ffxsam]

Sure.. but tomorrow morning :)

[i4got10]

Possible related to https://github.com/davidkpiano/react-redux-form/issues/65

[davidkpiano]

Good call, @i4got10! @ffxsam, try adding a displayName to the component:

import TextField from 'material-ui/...';

TextField.displayName = 'TextField'

I'll open a PR for this in material-ui if that solves the issue. They should ideally have a displayName property set.

[Rhywden]

I can submit success!

This indeed solves my issue with the empty model.

[davidkpiano]

Seems like that was the culprit, @ffxsam ! This will be addressed in v1.0 with the <Control> component (details here: https://github.com/davidkpiano/react-redux-form/issues/144#issuecomment-211973693 ), but for now, this workaround is quick and easy:

TextField.displayName = 'TextField';

[ffxsam]

Yikes - don't use displayName. It's an extra step that devs don't really want to have to add into their code. If xyz is a React element, use xyz.constructor.name.

[davidkpiano]

Yikes - don't use displayName. It's an extra step that devs don't really want to have to add into their code. If xyz is a React element, use xyz.constructor.name.

@ffxsam I know. This will be resolved in the upcoming v1.0. This is simply a workaround.

[ffxsam]

Oops sorry.. I missed the word "workaround" in your comment above 😏

[ffxsam]

Though I can't seem to figure out how to get the name of a stateless component.

[ffxsam]

I still don't understand what's causing this to break in production. displayName, which RRF relies on, doesn't even exist for me in dev.

[davidkpiano]

@ffxsam Without seeing your code it's a bit hard to tell... But can you try this?

<TextField onChange={(e) => dispatch(actions.change('foo.bar', e))} />

and see if that works in production?

[ffxsam]

Testing now..

[ffxsam]

Yes, I see foo.bar being changed in the Redux devtools.

[ffxsam]

An example of my form code:

            <MUITextField model="insightModel.title">
              <TextField
                floatingLabelText="Title"
                errorText={getError(fields, 'title')}
              />
            </MUITextField>

export const MUITextField = createFieldClass({
  TextField: props => ({
    onChange: props.onChange,
    value: props.modelValue,
  }),
});

[davidkpiano]

Okay, then I'm pretty sure that the issue is with the .displayName (or .constructor.name) not being visible in material-ui. It's unfortunate, but hey.

Manually adding the actions is a decent workaround, and all this will be solved with <Control>. It's good to know though that <Control> is basically just mapping the actions the same way that you would do it. It just provides a nicer API for doing so.

[ffxsam]

I still don't get why it breaks in production. If I inspect TextField in the Chrome console, $r.displayName and $r.constructor.displayName don't exist. And in your code, you're checking for names via:

    let controlDisplayName = control.constructor.displayName
      || control.type.displayName
      || control.type.name
      || control.type;

TextField has none of these properties, yet in dev it works fine. Am I missing something?

the issue is with the .displayName (or .constructor.name) not being visible in material-ui. It's unfortunate, but hey.

No, .constructor.name actually does work for MUI components:

(Chrome dev console)

» $r.constructor.name
« "TextField"

[davidkpiano]

Okay, I'm going to add the check for control.constructor.name and test this out locally to see if that fixes the problem.

[ffxsam]

@davidkpiano What did you find?

[davidkpiano]

Hosting a React hack a thon right now, nothing yet :)