dvlsg / async-csp

CSP style channels using ES7 async/await
MIT License
317 stars 18 forks source link

Library does not support ES5 style imports #7

Closed ivan-kleshnin closed 8 years ago

ivan-kleshnin commented 8 years ago
import Channel from "async-csp";
new Channel(); // ok

// vs

let Channel = require("async-csp");
new Channel(); // Channel is not a function

I suspect it's some Babel bug.

When I make this change in dist/channel.js

// exports['default'] = Channel;
module.exports = Channel;

it starts working. Maybe it's worth to update Babel version or something.


P.S. Actually, it's described here http://stackoverflow.com/questions/33505992/babel-6-changes-how-it-exports-default. I've never hit this behavior earlier. And it seems applicable to your version of Babel 5 as well.

dvlsg commented 8 years ago

Interesting. Definitely seems like an annoyance -- I'll see what I can do to circumvent the issue in babel, when I get a chance. Possibly with babel-plugin-add-module-exports or something similar (need to take a closer look).

dvlsg commented 8 years ago

How do you feel about using this when working with ES5/commonjs:

let Channel = require('async-csp').Channel;

I don't really like forcing users to use require('async-csp').default, but require('async-csp').Channel feels similar to a lot of ES5 modules I have used. If we took this route, this would allow all of the following to work, without modifying how ES6/7/babel can use library right now.

import Channel from 'async-csp';
import { Channel } from 'async-csp'; // new addition
let Channel = require('async-csp').Channel;
let Channel = require('async-csp').default;

But this would still not work:

let Channel = require('async-csp'); // still wouldn't be Channel

I could also potentially change it so it works with ES5/commonjs, but in order to make require('async-csp') === Channel I would have to muddy up the Channel class itself by adding things like STATES to the class definition. This wouldn't be terrible - I'm not 100% opposed to doing this, but right now I'm leaning towards the option detailed above.

petebacondarwin commented 8 years ago
import {Channel} from 'async-csp';

Is a very common approach.

Could one not do:

let {Channel} = require('async-csp');

when working with commonJS aswell?

dvlsg commented 8 years ago

Yes, that would work as well if I made the changes suggested above (assuming destructuring is available, anyways).

Still sitting on this one, since I haven't decided what to do yet. A lot of this issue is resulting from babel interoperability. I'm still leaning toward the changes I originally put forward as suggestions. I'll move forward with that shortly unless I hear any better suggestions.

dvlsg commented 8 years ago

The following imports should now work, as of commit fc2d063.

import Channel from 'async-csp';
let Channel = require('async-csp').default;
import { Channel } from 'async-csp'; // new addition
let Channel = require('async-csp').Channel; // new addition
let { Channel } = require('async-csp'); // new addition

Should land in npm as soon as I compile and push v0.4.1.