Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot import library with ES Modules #79

Open
knownasilya opened this issue Jun 29, 2016 · 12 comments
Open

Cannot import library with ES Modules #79

knownasilya opened this issue Jun 29, 2016 · 12 comments

Comments

@knownasilya
Copy link

knownasilya commented Jun 29, 2016

I'm using "babel-preset-es2015": "^6.9.0" with "babelify": "^7.3.0", and import sf from 'sheetify'; doesn't work (5.0.3 of sheetify). I get:

Uncaught TypeError: (0 , _sheetify2.default) is not a function
1.on-load @ google-map.js:4
s @ _prelude.js:1
(anonymous function) @ _prelude.js:12
../components/google-map @ index.js:3
s @ _prelude.js:1
e @ _prelude.js:1
(anonymous function) @ _prelude.js:1
@yoshuawuyts
Copy link
Contributor

idk man; import is barely useful as a spec, and I've got no idea what Babel's specific implementation quirks are - maybe someone else knows what's up; if you stumble upon a solution I reckon posting it back here would be greatly appreciated. Cheers!

@ahdinosaur
Copy link
Member

hey @knownasilya,

sheetify/transform only supports require, and i think babelify transpiles import into a non-statically-analyzable require (by wrapping it in a dynamic function) so that doesn't help.

so possible options:

  • change import sf from 'sheetify' to const sf = require('sheetify')
  • add support for ES Modules to sheetify/transform, so you could add it before babelify and it'd work.

i personally don't care for ES Modules, but if anyone wants to contribute here i'm keen.

@tj
Copy link

tj commented Jul 16, 2016

import { default as css } from 'sheetify' works fine for me, though definitely not as nice! kinda wonky to support both though

@yoshuawuyts
Copy link
Contributor

I'd strongly urge people to steer clear from import as it's less flexible, poorly defined (loader, anyone) and in its current form might not even make it to Node 😞 - way too much overhead for only syntax imo

@tj
Copy link

tj commented Jul 16, 2016

agreed, the syntax they came up with is pretty ridiculous but oh well haha

@knownasilya
Copy link
Author

@tj thanks for the tip

@mciparelli
Copy link
Contributor

@tj approach didn't work for me :(

@fczuardi
Copy link

Based on the outputs of rollup and babel for this file:

import sf from 'sheetify';
const loginCss = sf`
.login {
    position: absolute;
    width: 100%;
    height: 100%;
}
`;
console.log(loginCss);

Outputs before transform:

(rollup)

'use strict';

function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }

var sf = _interopDefault(require('sheetify'));

const loginCss = sf`
.login {
    position: absolute;
    width: 100%;
    height: 100%;
}
`;
console.log(loginCss);

(babel-cli)

'use strict';

var _sheetify = require('sheetify');

var _sheetify2 = _interopRequireDefault(_sheetify);

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

const loginCss = _sheetify2.default`
.login {
    position: absolute;
    width: 100%;
    height: 100%;
}
`;
console.log(loginCss);

One possible patch for the transform.js could be something like:

transform.js

--- left
+++ right
@@ -79,7 +79,18 @@
       node.arguments.length === 1 &&
       node.arguments[0].value === 'sheetify') {
         node.update('0')
-        mname = node.parent.id.name
+        // rollup
+        if (node.parent.callee && node.parent.callee.name === '_interopDefault') {
+          mname = node.parent.callee.parent.parent.id.name;
+        }
+        if (node.parent.id) {
+          // babel
+          if (node.parent.id.name === '_sheetify') {
+            mname = '_sheetify2';
+          }
+          // commonjs
+          mname = node.parent.id.name;
+        }
       }
     }

@@ -91,7 +102,9 @@
     function extractTemplateNodes (node) {
       if (node.type !== 'TemplateLiteral') return
       if (!node.parent || !node.parent.tag) return
-      if (node.parent.tag.name !== mname) return
+      if (node.parent.tag.name !== mname &&
+      !node.parent.tag.object &&
+      node.parent.tag.object.name !== mname) return

       const css = [ node.quasis.map(cooked) ]
         .concat(node.expressions.map(expr)).join('').trim()

@fczuardi
Copy link

if this should be implemented at all, or live under an option, or be a different project altogether is open for discussion ;)

@fczuardi
Copy link

the first solution proposed by @ahdinosaur

change import sf from 'sheetify' to const sf = require('sheetify')

Might be the simple solution, however acorn wont like files mixing import and require. ('import' and 'export' may appear only with 'sourceType: module'). To allow that, transform.js could be patched to use allowImportExportEverywhere: true option on the falafel calls.

--- left
+++ right
@@ -58,8 +58,8 @@
     var ast

     try {
-      const tmpAst = falafel(src, { ecmaVersion: 6 }, identifyModuleName)
-      ast = falafel(tmpAst.toString(), { ecmaVersion: 6 }, extractNodes)
+      const tmpAst = falafel(src, { ecmaVersion: 6, allowImportExportEverywhere: true }, identifyModuleName)
+      ast = falafel(tmpAst.toString(), { ecmaVersion: 6, allowImportExportEverywhere: true }, extractNodes)
     } catch (err) {
       return self.emit('error', err)
     }

@ngryman
Copy link

ngryman commented Jan 6, 2017

@yoshuawuyts What do you think about the last proposed solution of @fczuardi? I think it's a good compromise for only a small change.

@chinedufn
Copy link

chinedufn commented Apr 28, 2018

If you maintain something that uses ES Modules you can just fork sheetify and do something like this

https://github.com/chinedufn/sheetify/blob/master/transform.js#L82-L91

Then depend on your fork.

Assuming you have no other reasonable option and you're cool with all that comes with depending on a own fork instead of mainline.

sourceType: 'module' is the important bit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants