Tcomb-form-native: React Native Hot Reload breaks tcomb-form-native

[justim]

Version

• tcomb-form-native: 0.4.0
• react-native: 0.22.0-rc4
  

  Expected behaviour

React Native 0.22 introduces Hot Reload and after a code change the hot reload behaviour should not break the plugin.

Actual behaviour

After a code change the plugin throws an error.

Steps to reproduce

1. New react native project
2. Add a tcomb-form-native form
3. Enabled Hot Reload in the app
4. Make a change while the app is running
   

   Stack trace and console log

The error that is thrown:
[tcomb][tcomb-form-native] missing stylesheet config

Additional information

• The error is thrown in the Form.render method by the assert
• The stylesheet variable is indeed undefined after a code change
• The other asserts fail as well (templates and i18n)
• Hot Reload works by wrapping components with a shell that can reloads its component, so somehow the assignments in the index.js file are ignored

[dvisco]

It's not a fix but if you are looking for a workaround you can edit the render method of Form within lib/component.js to look like

    var type = this.props.type;
    var options = this.props.options || nooptions;
    var stylesheet = this.props.options.fields.name.stylesheet || Form.stylesheet;
    var templates = this.props.options.fields.name.templates || Form.templates;
    var i18n = this.props.options.fields.name.i18n || Form.i18n;

Then with your usage follow the example given in the stylesheet docs such as:

const t = require('tcomb-form-native');
var _ = require('lodash');

// clone the default stylesheet
const stylesheet = _.cloneDeep(t.form.Form.stylesheet);
const templates = _.cloneDeep(t.form.Form.templates);
const i18n = _.cloneDeep(t.form.Form.i18n);

let options = {
    fields: {
      name: {
        stylesheet: stylesheet, // overriding the style of the textbox
        templates: templates, // overriding the style of the textbox
        i18n: i18n // overriding the style of the textbox
      }
    }  
};

<Form ref="form"
    options={options}
     ...
/>

[adamrainsby]

Yep, this is an issue for me

[alvaromb]

Will look at this this weekend, thanks for reporting.

[rogerroelofs]

It seems like the hot reload process is somehow losing track of the t.form.Form modifications from /index.js. when I console.log(t.form.Form.stylesheet) from my code the stylesheet object is there, but when the form is actually constructed by the

If I add the requires to components.js and comment out the assignments in Form.render

var i18n = require('./i18n/en');
var templates = require('./templates/bootstrap');
var stylesheet = require('./stylesheets/bootstrap');

  render() {
    var type = this.props.type;
    var options = this.props.options || nooptions;
    //var stylesheet = Form.stylesheet;
    //var templates = Form.templates;
    //var i18n = Form.i18n;
    ...

it doesn't throw errors anymore but I don't know enough about hmr and tcomb-form-native internals to know if that is a good fix (and it doesn't feel right).

[alvaromb]

@gcanti should we move to import instead of require?

That could be breaking tcomb-form-native https://github.com/facebook/react-native/issues/5342#issuecomment-172398473

[gcanti]

@alvaromb since I wrote the first version of tcomb-form-native, ES6 support has improved. AFAIK we could move:

• var -> const and let
• require -> import
• module.exports -> export and export default

anything else?

[alvaromb]

I can check if moving from require to import solves the HMR issue, and then we could slowly start to move to ES6.

Enviado desde mi iPhone

El 25 mar 2016, a las 10:48, Giulio Canti [email protected] escribió:

@alvaromb since I wrote the first version of tcomb-form-native, ES6 support has improved. AFAIK we could move:

var -> const and let
require -> import
module.exports -> export and export default
anything else?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[justim]

Wouldn't it be a good idea to pass these properties as props to the component? The global assignment is not very react-like. And you could move the assignments in index.js to the default props.

This would of course break backwards compatibility, but if we do all the breaking things at once and release a new major version, all should be fine.

[gcanti]

@justim If I understand correctly you are proposing something like this (not tested):

// lib/components.js

...

class Form extends React.Component {

  ...

  render() {

    const type = this.props.type;
    const options = this.props.options || nooptions;
    const stylesheet = this.props.stylesheet; // <= read from props, was Form.stylesheet
    const templates = this.props.templates; // <= read from props, was Form.templates
    const i18n = this.props.i18n; // <= read from props, was Form.i18n

    t.assert(t.isType(type), `[${SOURCE}] missing required prop type`);
    t.assert(t.Object.is(options), `[${SOURCE}] prop options must be an object`);
    t.assert(t.Object.is(stylesheet), `[${SOURCE}] missing stylesheet config`);
    t.assert(t.Object.is(templates), `[${SOURCE}] missing templates config`);
    t.assert(t.Object.is(i18n), `[${SOURCE}] missing i18n config`);

    ...

  }

}

// index.js

import t from './lib';
import i18n from './lib/i18n/en';
import templates from './lib/templates/bootstrap';
import stylesheet from './lib/stylesheets/bootstrap';

t.form.Form.defaultProps = {
  templates
  stylesheet
  i18n
};

export default t;

Maintaining backward compatibility?

// index.js

import t from './lib';
import i18n from './lib/i18n/en';
import templates from './lib/templates/bootstrap';
import stylesheet from './lib/stylesheets/bootstrap';

t.form.Form.templates = templates;
t.form.Form.stylesheet = stylesheet;
t.form.Form.i18n = i18n;

t.form.Form.defaultProps = {
  templates: t.form.Form.templates
  stylesheet: t.form.Form.stylesheet
  i18n: t.form.Form.i18n
};

export default t;

[justim]

Yes, indeed, something like that. But that alone would not solve the HMR issue, because it's an global assignment and will probably break in the same way the old version did. But we could test that.

// lib/components.js

import i18n from './i18n/en';
import templates from './templates/bootstrap';
import stylesheet from './stylesheets/bootstrap';

class Form extends React.Component {
    static defaultProps = {
        i18n,
        templates,
        stylesheet,
    };

    ...

    // or maybe something like this to maintain backward compatibility
    static defaultProps = {
        i18n: Form.i18n || i18n,
        templates: Form.templates || templates,
        stylesheet: Form.stylesheet || stylesheet,
    };
}

A problem with maintaining backward compatibility might be that it never (?) work with HMR. But again, we can test this. :)

[alvaromb]

I'm a complete ignorant about how HMR works, thus I don't know exactly why it breaks the library. It's just the global assignment? I'll be more than happy to code a solution this weekend, but I would like to understand better the issue first.

Speaking about backwards compatibility, in my opinion tcomb-form-native should be tightly coupled to RN in terms of updates, so top priority right now should be make it work to latest version until RN 1.0 lands. 0.4 it's compatible only with latest RN versions by the way. Regardless of my opinion, @gcanti is the creator of the library and he must validate that decision (and define a backward compatibility policy for the lib). These are my two cents, but of course I would like to hear different opinions.

Will be working on this starting tomorrow (it's holiday in Spain right now), but expect a fix very soon because we need to update some of our apps to 0.22/23 this week.

Enviado desde mi iPhone

El 25 mar 2016, a las 12:09, Tim [email protected] escribió:

Yes, indeed, something like that. But that alone would not solve the HMR issue, because it's an global assignment and will probably break in the same way the old version did. But we could test that.

// lib/components.js

import i18n from './i18n/en';
import templates from './templates/bootstrap';
import stylesheet from './stylesheets/bootstrap';

class Form extends React.Component {
static defaultProps = {
i18n,
templates,
stylesheet,
};

...

// or maybe something like this to maintain backward compatibility
static defaultProps = {
    i18n: Form.i18n || i18n,
    templates: Form.templates || templates,
    stylesheet: Form.stylesheet || stylesheet,
};

}
A problem with maintaining backward compatibility might be that it never (?) work with HMR. But again, we can test this. :)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[alvaromb]

Sorry guys I have two important deadlines this week, will try to work on this ASAP.

[rturk]

@alvaromb looks like that there are a few changes in the HMR module in the upcoming RN releases. It will be interesting to check Release Candidate for RN .23 and RN .24, maybe this will go away in newer releases

[adamrainsby]

@rturk I've just upgraded to 0.23.0 and the problem persists. Haven't tried the 0.24 RCs.

[gcanti]

Relevant https://github.com/gcanti/tcomb-form-native/pull/139

[rturk]

@adamrainsby You're correct. I've also upgraded to RN 0.23 problem persists

[adamrainsby]

I was hoping react native 0.24.0 would fix it because of the release notes but I just upgraded and it still fails.

[mtven]

Hey guys
Any updates here?

[thomasjonas]

I'm on 0.25.1 now and it still fails. Would be really nice to see this fixed!

[justim]

When I got time this week I'm happy to work on it a bit and create a PR. Or have you started it already, @gcanti?

[alvaromb]

@justim I would love some feedback here https://github.com/gcanti/tcomb-form-native/pull/139

I can update the PR this week.

[justim]

Even better, I'll have a look at it :)

[justim]

Great 👍

[alvaromb]

Should we stick something in the docs @gcanti ?

[gcanti]

@alvaromb if that fix works and is really backward compatible (as I hope) I'd say no

[gcanti]

Released a fix in https://github.com/gcanti/tcomb-form-native/releases/tag/v0.4.3, please check it out (thanks all for your help).

[sirwoetang]

For those on the same low level as I am, it seems that the import implementation:

var t = require('tcomb-form-native')
var Form = t.form.Form

does not work as of 0.4.3

[adamrainsby]

@sirwoetang I'm also having that problem
"Cannot read property 'Form' of undefined"

[alvaromb]

I can't reproduce the issue, can you provide me more info? Thanks!

[alvaromb]

I'm currently using 0.4.3 with a form just like this

import React from 'react'
import t from 'tcomb-form-native'

const Form = t.form.Form

[rogerroelofs]

What version of RN are you using? Do you have a custom babelrc file?

[adamrainsby]

@rogerroelofs I am using 0.24.0 and I do not have a custom babelrc file

[justim]

The problem is with the new way of export the module:

export default t;

// is sugar for
module.exports.default = t;

// you would need to do this to use it
var t = require('tcomb-form-native').default;

[adamrainsby]

@justim Thanks!

[justim]

I think the module needs to change, not _your_ code, @adamrainsby :)

This is quite a breaking change. I created a PR for it (#160).

[sirwoetang]

@rogerroelofs I am using 0.25.1 and I do not have a custom babelrc file

[alvaromb]

Sorry about this, I'm no expert about how the modules work, I come from the objc world. Please let me know if you need anything!

Enviado desde mi iPhone

El 10 may 2016, a las 18:34, sirwoetang [email protected] escribió:

@rogerroelofs I am using 0.25.1 and I do not have a custom babelrc file

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

[gcanti]

Thanks @justim, new release on its way...

[gcanti]

v0.4.4 released

[gcanti]

Hey guys, is everything ok now?

Sorry @alvaromb, the wrong export was my fault.

[adamrainsby]

@gcanti Yep works great. Thanks

[justim]

@gcanti works as a charm, hot reload and all 👍

[alvaromb]

Yes, it's working great!

It's ok @gcanti, I didn't catch it either 😆

And thanks for your help @justim & others!

[jshelton31]