Tighten .npmignore ; use babel as transpiler

[wbazant]

Thanks for the package! We have a problem of an expensive component with an svg, that we'd like to redraw asynchronously, with an extra state change after the rest of our components are already in place, and this package addresses it.

This change proposes to use Babel to create the lib/index.js instead of webpack.

The resulting file is quite short, I am pasting it below - for comparison, the current webpack output has 576 lines.

It expects the client of the package to have a build process involving something like webpack that will understand what to do with e.g. require('react'), but it's a fairly safe assumption for react projects.

I've ran the tests and they passed but I had to locally make a following change:

diff --git a/e2e/src/App.js b/e2e/src/App.js
index 2cc4c71..8b0ae5a 100644
--- a/e2e/src/App.js
+++ b/e2e/src/App.js
@@ -1,5 +1,5 @@
 import React, { Component } from 'react';
-import debounceRender from 'react-debounce-render';
+import debounceRender from '../../lib/index.js';

otherwise the tested code wasn't the newest version :)

Here's how the output looks like:

var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();

var _react = require('react');

var _react2 = _interopRequireDefault(_react);

var _lodash = require('lodash');

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

function _possibleConstructorReturn(self, call) { if (!self) { throw new ReferenceError("this hasn't been initialised - super() hasn't been called"); } return call && (typeof call === "object" || typeof call === "function") ? call : self; }

function _inherits(subClass, superClass) { if (typeof superClass !== "function" && superClass !== null) { throw new TypeError("Super expression must either be null or a function, not " + typeof superClass); } subClass.prototype = Object.create(superClass && superClass.prototype, { constructor: { value: subClass, enumerable: false, writable: true, configurable: true } }); if (superClass) Object.setPrototypeOf ? Object.setPrototypeOf(subClass, superClass) : subClass.__proto__ = superClass; }

module.exports = function debounceRender(ComponentToDebounce) {
    var _class, _temp, _initialiseProps;

    for (var _len = arguments.length, debounceArgs = Array(_len > 1 ? _len - 1 : 0), _key = 1; _key < _len; _key++) {
        debounceArgs[_key - 1] = arguments[_key];
    }

    return _temp = _class = function (_Component) {
        _inherits(DebouncedContainer, _Component);

        function DebouncedContainer(props) {
            _classCallCheck(this, DebouncedContainer);

            var _this = _possibleConstructorReturn(this, (DebouncedContainer.__proto__ || Object.getPrototypeOf(DebouncedContainer)).call(this, props));

            _initialiseProps.call(_this);

            _this.state = props;
            _this.shouldRender = false;
            return _this;
        }

        _createClass(DebouncedContainer, [{
            key: 'componentWillReceiveProps',
            value: function componentWillReceiveProps(props) {
                this.shouldRender = false;
                this.updateState(props);
            }
        }, {
            key: 'shouldComponentUpdate',
            value: function shouldComponentUpdate() {
                return this.shouldRender;
            }
        }, {
            key: 'render',
            value: function render() {
                return _react2.default.createElement(ComponentToDebounce, this.state);
            }
        }]);

        return DebouncedContainer;
    }(_react.Component), _initialiseProps = function _initialiseProps() {
        var _this2 = this;

        this.updateState = _lodash.debounce.apply(undefined, [function (props) {
            _this2.shouldRender = true;
            _this2.setState(props);
        }].concat(debounceArgs));
    }, _temp;
};

[podefr]

Thanks for the PR! I'll review it and give you my feedback asap.
What's the main reason for using babel instead of webpack?

[wbazant]

Babel is a transpiler, it produces files in plain Javascript so that users of the library don't need to support the same language level - that lets the library use features the language doesn't normally have, e.g. I saw this package does have babel at stage-0 to enable class methods. Webpack is not designed as a transpiler, it's for something else: it's for producing dist files that are served to the client.

As a consequence the current "lib" file has a baked in copy of lodash.debounce unnecessarily and some webpack module setup, much like a "dist" file for a whole application typically would have.

[podefr]

Yes, I guess I just wanted to understand precisely what problem this PR is solving. I understand the "how", not the "why":

• is it to remove some boilerplate from the lib file after it's built and extract lodash debounce as a dependency
• or did you encounter a language level issue whereby the module can't be used?

Based on that I'll be better able to assess what I want to keep from the PR :)

[podefr]

btw, agreed that the e2e don't run the current version but rather the version present in e2e/node_modules. I did that on purpose so that I could not only test the code but also ensure that my package.json is properly configured. It's the closest thing to ensuring that the published module works as intended.

I agree though that the prepublish task doesn't make much sense anymore, I should probably update it to npm pack/npm install the current version of the library before running the tests

[wbazant]

The first why, and wanting nice things to exist, and wanting to learn something. We have tried to settle on a standard for our packages internally and the best way to know I'm wrong is to be confident about something on the internet ;)

How I got here: I did something similar as a POC, it seems to have worked, then I thought I'd try use a community standard solution and I found react-debounce-render. I'm still hesitant to bring it in - I think I'll get warnings about why am I using a dist version of a package, I don't want to have duplicate lodash.debounce, and there doesn't yet seem to be much community which will find bugs and corner cases, update when next react version comes out etc.

Re tests - it's a pain to have tests as a separate project, our setup is just a test folder and import ReactDebounceRender from "../src/index.js" there. After you use an npmignore from this PR ;) it won't make it into the published package.

[podefr]

Thanks for the suggested change.

I liked that the project was standalone and didn't make any assumption on the presence of a build tool but as you said it's probably unnecessary as most react projects will have one anyway. The resulting lib and npm package are much cleaner, so it's a change for good.

I left 3 minor comments on your PR, can you take a look? I'll release a v2.0.0 when your PR is merged.

[podefr]

thanks for introducing the .npmignore. Since the configuration is self explanatory the comments don't seem necessary, could you remove them?

[podefr]

could you add spaces inside {} for consistency? { debounce }

[podefr]

why is .* necessary since we already have *? can it be removed?

[wbazant]

Done! I've accidentally signed the commit with my twin brother's account, let me know if it matters and I'll try to undo it.
You're right about the .npmignore not needing the comment, in https://github.com/gxa/atlas-package/blob/master/.npmignore we're really spelling it out :)

[podefr]

no worries about the commit author, the more the merrier.
I'll merge your PR and publish version 2.0.0.

thanks a lot for your contribution!