Skip to content

Commit

Permalink
Improve performance by removing pitch loader, also disable notifications
Browse files Browse the repository at this point in the history
  • Loading branch information
gaearon committed Aug 21, 2014
1 parent 37edb09 commit 02c9713
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 121 deletions.
3 changes: 1 addition & 2 deletions .jshintrc
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"node": true,
"globals": {
"window": true,
"Notification": true
"window": true
}
}
12 changes: 8 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ Hot reload is disabled for modules that contain no `React.createClass` calls and

Several components in one file will work as long as their `displayName`s are different.

### Options

* `notify`: Loader can use desktop Notification API to show notifications when a module has been reloaded, or if it loads with an error. By default, this feature is disabled because it doesn't work well with `webpack-dev-server` iframe mode used in the example. If you don't use `webpack-dev-server`'s iframe mode, you might want to enable notifications. Valid values are `none` (default), `errors` and `all`.

## Running Example

```
Expand All @@ -63,6 +59,14 @@ A better approach may be to make monkeypatch `createClass` to return a proxy obj
## Changelog

#### 0.4.0

* Ignore files that contain no `createClass` calls (fixes **[#17]**(https://github.com/gaearon/react-hot-loader/issues/17))
* Remove the need for pitch loader (fixes **[#19](https://github.com/gaearon/react-hot-loader/issues/19)**)
* Improve performance by only using one loader instead of two
* Now that performance is acceptable, remove desktop notifications and `notify` option
* It is now recommended that you use `devtool: 'eval'` because it's much faster and has no downsides anymore

#### 0.3.1

* Avoid warnings on old browsers with missing `Notification` API
Expand Down
19 changes: 19 additions & 0 deletions getHotUpdateAPI.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

var updaters = {},
makeModuleUpdater = require('./makeModuleUpdater');

function getHotUpdateAPI(React, filename, moduleId) {
var exists = updaters.hasOwnProperty(moduleId);
if (!exists) {
updaters[moduleId] = makeModuleUpdater(React, filename);
}

var updater = updaters[moduleId];
return {
createClass: exists ? updater.updateClass : updater.createClass,
updateMountedInstances: updater.updateMountedInstances
};
}

module.exports = getHotUpdateAPI;
50 changes: 26 additions & 24 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,34 @@
var path = require('path'),
loaderUtils = require('loader-utils');
var path = require('path');

module.exports = function () {};
module.exports.pitch = function (remainingRequest) {
this.cacheable && this.cacheable();
module.exports = function (source) {
if (this.cacheable) {
this.cacheable();
}

var patchedModuleRequest = '!!' + require.resolve('./replaceCreateClass') + '!' + remainingRequest,
originalFilename = path.basename(remainingRequest),
query = loaderUtils.parseQuery(this.query);
var matches = 0,
processedSource;

query.notify = query.notify || 'none';
processedSource = source.replace(/React\.createClass/g, function (match) {
matches++;
return '__hotUpdateAPI.createClass';
});

return [
'var React = require("react");',
'var notifier = require(' + JSON.stringify(require.resolve('./makeNotifier')) + ')(' + JSON.stringify(originalFilename) + ', ' + JSON.stringify(query.notify) + ');',
'var moduleUpdater = require(' + JSON.stringify(require.resolve('./makeModuleUpdater')) + ')(' + JSON.stringify(originalFilename) + ', React);',

'module.exports = require(' + JSON.stringify(patchedModuleRequest) + ')(moduleUpdater.createClass);',
if (!matches) {
return source;
}

'if (module.hot && moduleUpdater.canUpdateModule() && React.isValidClass(module.exports)) {',
' module.hot.accept(' + JSON.stringify(patchedModuleRequest) + ', function () {',
' try {',
' require(' + JSON.stringify(patchedModuleRequest) + ')(moduleUpdater.updateClass);',
' moduleUpdater.updateMountedInstances();',
' notifier.handleSuccess()',
' } catch (err) {',
' notifier.handleFailure(err)',
' }',
return [
'var __hotUpdateAPI = (function () {',
' var React = require("react");',
' var getHotUpdateAPI = require(' + JSON.stringify(require.resolve('./getHotUpdateAPI')) + ');',
' return getHotUpdateAPI(React, ' + JSON.stringify(path.basename(this.resourcePath)) + ', module.id);',
'})();',
processedSource,
'if (module.hot) {',
' module.hot.accept();',
' module.hot.dispose(function () {',
' var nextTick = require(' + JSON.stringify(require.resolve('next-tick')) + ');',
' nextTick(__hotUpdateAPI.updateMountedInstances);',
' });',
'}'
].join('\n');

This comment has been minimized.

Copy link
@sokra

sokra Aug 22, 2014

You should consider using the source-map.SourceNode class to build the source. This is required to support SourceMaps once petehunt/jsx-loader#16 get merged...

This comment has been minimized.

Copy link
@gaearon

gaearon Aug 22, 2014

Author Owner

Yeah, I'm waiting for JSX first. I'll definitely do that as soon as that's merged.

Expand Down
9 changes: 2 additions & 7 deletions makeModuleUpdater.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* replacement for `React.createClass` in a module. If multiple components
* are defined in the same module, assumes their `displayName`s are different.
*/
module.exports = function (filename, React) {
module.exports = function (React, filename) {
var componentUpdaters = {};

function createClass(spec) {
Expand Down Expand Up @@ -40,14 +40,9 @@ module.exports = function (filename, React) {
});
}

function canUpdateModule() {
return Object.keys(componentUpdaters).length > 0;
}

return {
createClass: createClass,
updateClass: updateClass,
updateMountedInstances: updateMountedInstances,
canUpdateModule: canUpdateModule
updateMountedInstances: updateMountedInstances
};
};
71 changes: 0 additions & 71 deletions makeNotifier.js

This file was deleted.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-hot-loader",
"version": "0.3.1",
"version": "0.4.0",
"description": "Webpack loader that enables live-editing React components without unmounting or losing their state",
"main": "index.js",
"scripts": {
Expand All @@ -10,7 +10,7 @@
"example": "example"
},
"dependencies": {
"loader-utils": "^0.2.3"
"next-tick": "0.2.2"
},
"devDependencies": {
"jsx-loader": "0.11.0",
Expand Down
11 changes: 0 additions & 11 deletions replaceCreateClass.js

This file was deleted.

0 comments on commit 02c9713

Please sign in to comment.