search

ava-ia / core

Agnostic Virtual Assistant
219 stars 21 forks source link

Add babel-plugin-transform-runtime #11

Closed jhnns closed 8 years ago

jhnns commented 8 years ago

Thx for this great module.

There is a small issue and I'm not sure if this is intended. If I just install the module from npm and execute it in node without using babel, it throws 

ReferenceError: regeneratorRuntime is not defined
    at /node_modules/ava-ia/lib/helpers/factoryActions.js:10:31
    at Object.<anonymous> (/node_modules/ava-ia/lib/helpers/factoryActions.js:36:2)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/node_modules/ava-ia/lib/helpers/index.js:43:23)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.require (module.js:468:17)

This is because the compiled code requires some global variables, like regeneratorRuntime which you need to polyfill if you want to use the module without babel.

I've added the babel-plugin-transform-runtime which translates these global dependencies into local requires. In this case, you don't need to polyfill the runtime environment anymore (like you did in the index.js).

You'll need to build the code again of course. I haven't done that in this PR because I did not want to pollute it with generated code.

soyjavi commented 8 years ago

Check how you can use it on https://github.com/ava-ia/node-example

jhnns commented 8 years ago

Yes, I know how to use it. This is the way how I've currently "fixed" it in my project. Now I have a dependency on babel although I don't use it.

This might be subjective but I'm pretty sure that many developers in the NPM community would consider it bad practice to publish a module which requires global variables to be polyfilled. My PR suggests a change, that users of your module would not need to use babel at all. Your module would be completely self-contained and everyone could use it without needing to install babel.

haganbt commented 8 years ago

+1

I hit this issue when testing. This fix looks ideal.

soyjavi commented 8 years ago

🖖