Fix crash on encountering import/export syntax

[dmotz]

This fix addresses #30 as a step toward supporting import/export syntax.

While it doesn't update yo-yoify's transformer to support this syntax, it allows codebases that use ES6 style modules to use it without crashing.

The current version will crash with:

SyntaxError: 'import' and 'export' may appear only with 'sourceType: module'

To other users interested in using this transform with ES6 modules, this change will allow a stopgap solution which is to use import statements, but switch to require when using a yo-yoify supported function like choo/html.

[yoshuawuyts]

Nice! Question tho: what are the side effects of setting that parameter? e.g. does it mean that all modules are now interpreted as ES modules and stuff like strict mode automatically applies?

Perhaps adding a test could also be helpful (:

[dmotz]

Great point. Looking at the source for Acorn, it does seem that sourceType: 'module' will enable strict mode, but I found another option called allowImportExportEverywhere that seems to be a kinder, gentler way of allowing import statements (see here) so I changed it to that.

The problem with the tests is that Browserify itself chokes on import once it calls bundle even though the yo-yoify transform went through ok. We could add another transform in to make the test case work but I'm not sure if you're open to that.

I've tried transforming the ES6 module syntax into common prior to yo-yoify to avoid this whole issue, but it seems that most of the transform modules out there (e.g. Babel, Rollup) do weird things that throw off yo-yoify, like:

html = require('choo/html');
html2 = html;
module.exports = html2`<div></div>`;
// yo-yoify no longer recognizes this as a transform target

Sorry if this is starting to seem only tangentially related to the project. I'm not fully sold on ES6 modules, but they do have some advantages and many projects have adopted them. Was excited to see this transform working since it definitely speeds things up.

Let me know if you have any more thoughts on this.

[ruudud]

:+1: on using allowImportExportEverywhere. It's impossible to use yo-yoify with babel niceness at the moment.

What's blocking this? A missing test?

[yoshuawuyts]

It's been a while since I last looked at this; given it doesn't have any added value for me I don't really feel like investing any time into making sure this works. If someone writes a thoroughly tested patch that clearly describes tradeoffs and edge cases I think we can merge it, but yeah otherwise things are going to remain just the way they are (:

On Fri, Feb 10, 2017 at 5:26 PM Pål Ruud notifications@github.com wrote:

👍 on using allowImportExportEverywhere. It's impossible to use yo-yoify with babel niceness at the moment.

What's blocking this? A missing test?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/shama/yo-yoify/pull/32#issuecomment-278990477, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWlevW5rZ3c2yJou5BG_CDLBqFaFVwmks5rbI_SgaJpZM4LKEov .

[ruudud]

Totally understand. 👍

We ended up rewriting our ES6 import statements to commonjs require(). Browserify is basically "Node imports in the browser", and I guess once ES6 imports gets into Node this issue might be addressed.

FWIW, our browserify build is now something like

NODE_ENV=production browserify -t envify -t unassertify -t yo-yoify -t [ babelify --presets [ es2015 ] ] -t uglifyify index.js | uglifyjs -c -- - > bundle.js

(no global transforms or uglify mangling because of badly behaving dependencies).

For us, it's important that yo-yoify is run before babelify, because we're hitting this: https://github.com/shama/bel#note

This PR might however be a quick win to achieve the same with ES6 imports though, but I understand that you're reluctant to include it for the reasons you mention.

[dmotz]

This change will just allow basic compatibility for those trying out import statements in their source, with selective use of require for yo-yo (or choo/html).

While the original version of this PR enabled used sourceType: 'module' to fix this, the latest simply uses allowImportExportEverywhere: true which is limited to a single use in Acorn's source here: https://github.com/ternjs/acorn/blob/e975ebdd4898362e4b4242a6503e2582acd32f95/src/statement.js#L106

That throw statement there is what blocks using yo-yoify at all in any project with an import statement -- this PR just quietly avoids that without any other side effects.

Hope that clarifies more what the aim is.