webpack issue "import and export may only appear at the top level"

[babakness]

npm run prepublish doesn't produce the same index.js in dist that one gets from npm install rethinkdb-webdocket-client. The version of dist/index.js via npm install is in es5, the one produced by the local webpack config is in es6 with stated issues. No biggie but seems like an update to scripts in package.json makes sense.

[babakness]

Would be really helpful for contributors to be on the same page. Not exactly sure why but it seems that babel doesn't preprocess properly. I've got my own thing working though it seems that in the future I'd have to import that tooling. Would be great if this library's build system produced the same output dist/index.js out-the-box.

git clone [this project]
npm install
npm run prepublish

Produces

Hash: acc876bcf1d0ddb8df53
Version: webpack 1.13.2
Time: 67ms
   Asset     Size  Chunks             Chunk Names
index.js  2.79 kB       0  [emitted]  main
   [0] multi main 28 bytes {0} [built]
    + 1 hidden modules
Hash: 0e05435d02fa53476bb7
Version: webpack 1.13.2
Time: 71ms
  Asset     Size  Chunks             Chunk Names
node.js  2.79 kB       0  [emitted]  main
   [0] multi main 28 bytes {0} [built]
    + 1 hidden modules

2.79kb.

(function webpackUniversalModuleDefinition(root, factory) {
... webpack stuff ...
    module.exports = __webpack_require__(1);


/***/ },
/* 1 */
/***/ function(module, exports) {

    import Promise from 'bluebird';
    import rethinkdb from 'rethinkdb';
    import protodef from 'rethinkdb/proto-def';
    import {configureTcpPolyfill} from './TcpPolyfill';

    function connect({host, port, path, secure, wsProtocols, db, simulatedLatencyMs}) {
      configureTcpPolyfill({path, secure, wsProtocols, simulatedLatencyMs});
      // Temporarily unset process.browser so rethinkdb uses a TcpConnection
      const oldProcessDotBrowser = process.browser;
      process.browser = false;
      const connectOptions = {host, port, db};
      const connectionPromise = Promise.promisify(rethinkdb.connect)(connectOptions);
      process.browser = oldProcessDotBrowser;
      return connectionPromise;
    }

    const RethinkdbWebsocketClient = {
      rethinkdb,
      protodef,
      Promise,
      connect,
    };

    export {
      rethinkdb,
      protodef,
      Promise,
      connect,
      RethinkdbWebsocketClient as default,
    };


/***/ }
/******/ ])
});

Babel didn't operate.

[babakness]

Hmm.. seems that in this line

https://github.com/mikemintz/rethinkdb-websocket-client/blob/master/webpack/base.config.js#L19

removing exclude would work.

In what scenario do we want exclude for npm run prepublish?

[mikemintz]

@babakness, I'm using the latest master, and I don't think I'm seeing the same behavior as you. I.e. I clone this project from git, run npm install and npm run prepublish, and the resulting code in dist is ES5, whereas you are seeing ES6?

$ git clone [email protected]:mikemintz/rethinkdb-websocket-client.git
Cloning into 'rethinkdb-websocket-client'...
[etc...]

$ cd rethinkdb-websocket-client/

$ npm install
npm WARN deprecated [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
[etc...]

$ npm run prepublish
> [email protected] prepublish /home/mike/Downloads/rethinkdb-websocket-client
[etc...]

$ grep -r 'const ' dist
$ grep -r 'import ' dist

[babakness]

Not sure why. It seems that the main difference might be the version of Node? Also, I'm on a Mac.

After I removed exclude: ... the linter started complaining about trailing spaces in my comments. I made the modifications to keep it happy.

In my private fork I included the dist folder in the repo. It makes hands-on testing easier. The developer can simply edit package.json to point to a remote tarball as the version number and then switch it to a specific/minimal version when npm gets updated.

https://github.com/babakness/rethinkdb-websocket-client

[mikemintz]

I'm using node 6.5.0 on linux, but I have also gotten it working on node 0.12, node 4, and node 5.

Does your dist/node.js file also have ES6, or is it only dist/index.js? The unit tests in npm test should also fail for you if dist/node.js is ES6.

I saw the trailing spaces issue as well and made a commit to fix that, but I don't think that made a difference for the ES5/6 issue on my end.