Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting close to 1.0 #44

Closed
4 tasks done
gaearon opened this issue Dec 12, 2014 · 5 comments
Closed
4 tasks done

Getting close to 1.0 #44

gaearon opened this issue Dec 12, 2014 · 5 comments

Comments

@gaearon
Copy link
Owner

gaearon commented Dec 12, 2014

Preliminary 1.0 is released on NPM as 1.0.0-alpha.3.

It adds support for ES6 classes (against React master), fixing #39.

It also fixes #24 via opt-in Component = module.makeHot(Component, uniqueId?) that you can use even inside a function. (The default, however, has changed to only “hotify” module.exports.)

Some similar issues are solved as a side effect (#43, #33).

Known issues:

  • Autobinding of newly added methods doesn't work against React's master
  • When component is unmounted, its methods may keep firing, potentially causing exceptions
  • It's slightly awkward to type if (module.makeHot) X = module.makeHot(X), can we think of a better API (note that this is only needed if you want to hotify something other than module.exports)? We also want to make sure no one ever forgets the check so it doesn't crash in production—maybe noop would be better.
  • Update docs, examples and blog entry

You can help by doing npm install --save-dev react-hot-loader@1.0.0-alpha.3 and reporting issues in your projects!

@natew
Copy link
Contributor

natew commented Dec 12, 2014

It works! Initial testing is looking awesome.

@gaearon
Copy link
Owner Author

gaearon commented Dec 12, 2014

I thought more about makeHot and I would rather have it more explicit:

// You'd need to require it and pass `module` object to it
var makeHot = require('react-hot-loader/makeHot')(module);

// if you need to make hot something other than module.exports, just wrap it:
var Something = makeHot(React.createClass({
  ...
});

// also works for classes:
class SomethingElse {
  ...
}
SomethingElse = makeHot(SomethingElse);

This is more explicit than module.makeHot, but it allows to eliminate if condition because in production, we'll just return whatever you passed as an argument.

For comparison, the syntax I considered before was more magical but also more error-prone:

var Something = React.createClass({
  ...
};

if (module.makeHot) {
  Something = module.makeHot(Something);
}

class SomethingElse {
  ...
}

if (module.makeHot) {
  SomethingElse = module.makeHot(SomethingElse);
}

The check was necessary because we wouldn't have module.makeHot in production (whereas an exported function would just be a no-op returning its argument).

Thoughts?

(This is all only relevant for components that aren't module.exports.)

Edit: Now that I think of it, wrapping isn't really helpful because JSX wouldn't figure out displayName and we'd have to pass second parameter to makeHot every time.

@natew
Copy link
Contributor

natew commented Dec 18, 2014

So I split up an old project into some smaller pieces, and with the app I've been running I started getting this error that I was having trouble debugging. Lo and behold disabling hot components fixes it, so I'm guessing it has something to do with react-hot-loader. Unfortunately it probably won't be open sourced for another month or so.

But, I can upload info on the error and setup. I actually had it fixed at one point because I was loading multiple versions of React, but I've triple checked that now and it seems my bundle is clean.

This is with react 0.12.1, hot-loader alpha.3.

Error comes on whenever I first require('react'):

Uncaught TypeError: undefined is not a function
ReactEmptyComponent.js:25

That line is this:

var ReactEmptyComponentInjection = {
  injectEmptyComponent: function(emptyComponent) {
    component = ReactElement.createFactory(emptyComponent);
  }
};

ReactElement is an empty object for some reason, even though when looking through the requires, it seems it's loading the file that sets up all those functions like createFactory.

Anyway, if I disable it seems to work. I'll try rolling back maybe and seeing if it's the latest version that causes this.

@gaearon
Copy link
Owner Author

gaearon commented Dec 19, 2014

Whew, that's quite weird, sounds like some kind of dependency order issue.

Can you please verify that neither react-hot-loader nor react-hot-loader/node_modules/react-hot-api have their own Reacts? (I know you checked but I still wonder about react-hot-api somehow being involved)

@gaearon
Copy link
Owner Author

gaearon commented Dec 19, 2014

@natew I split this into a separate issue: #47

Thanks!

@gaearon gaearon closed this as completed Dec 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants