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

JSPM + Riot.js + import statements in tags #1715

Closed
1 of 7 tasks
josephrocca opened this issue Mar 31, 2016 · 14 comments
Closed
1 of 7 tasks

JSPM + Riot.js + import statements in tags #1715

josephrocca opened this issue Mar 31, 2016 · 14 comments
Labels

Comments

@josephrocca
Copy link

Describe your issue:

I'm trying to get riot.js to work with JSPM and it was all going perfect until I put an import statement inside the script tag in one of my custom riot tags.

I get this error: Uncaught (in promise) Error: SyntaxError: Unexpected token import system.js:4

My question is: Is this a problem with riot, JSPM, me, or some combination?

JSPM has been working great for me so far so I don't think that's the problem. I've seen other examples where putting import statements inside script tags works fine, so I don't think it's a problem with riot.

So maybe there's something simple I'm missing? Someone suggested to install babel-preset-es2015, but I think JSPM already handles all the babel stuff so that's not relevant to me.

Can you reproduce the issue?

I don't think so unless plunker supports JSPM. I made an example of what I'm trying to do in any case:
http://plnkr.co/edit/80FlhIABVnfTv7mops2I?p=preview

EDIT: I made a repo showing the problem: https://github.com/josephrocca/riot-jspm/tree/import_in_tag_test

Just need to call npm install and then jspm install and it's good to go.

On which browser/OS does the issue appear?

Chrome, Windows

Which version of Riot does it affect?

2.3.17

How would you tag this issue?

  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@planeguy
Copy link

JSPM still uses Babel 5, so babel-preset-es2015 isn't going to help.

I'm getting the same issue, by the way. I workaround it by importing all my components before mounting, but that's not really the same thing, especially when you've got stuff that are sub components of other components. should be the only thing that needs to import if you know what I mean.

@josephrocca
Copy link
Author

I'm not sure I understand what you mean - you import all your components in main.js (or equivalent)? If so, I guess that'd work for very simple projects.

So is this a problem with with JSPM then? I wouldn't have thought so since it does its job fairly smoothly without riot.

@planeguy
Copy link

planeguy commented Apr 1, 2016

I would also maybe check with systemjs-riot.

I don't know jspm very well but if we could do a precompile the tags then import the js it'd probably work fine.

@josephrocca
Copy link
Author

Yeah I really have no idea with this. I hoping this won't stop me from using riot, because it's been really fun so far.

@guybedford please ignore this (and sorry for the ping) if you're busy: can you make any comment on riot from JSPM's point of view? It seems like a very promising framework and it'd be great for both JSPM and riot if they could get friendly with one another.

@guybedford
Copy link

Sure, I just had a brief look at this. So systemjs-riot is compiling assuming that tags are run as AMD modules - https://github.com/HuasoFoundries/systemjs-riot/blob/master/tag.js#L10.

You can alter that to output ES modules with:

var compiler = require('riot-compiler');

exports.translate = function(load) {
    // @todo be able to pass the options in config.js
    var options = {
        //expr: true
        //type: 'babel'
    };

    load.metadata.format = 'esm';
    var compiledtag = compiler.compile(load.source, options);
    //    console.log(compiledtag);
    var output = "import riot from 'riot';\n" + compiledtag;

    return output;
};

But you then get a Babel error:

undefined:1 Uncaught (in promise) Error: SyntaxError: file:///Users/guybedford/Projects/test/riot-jspm/app.tag: 'import' and 'export' may only appear at the top level (3:4)
      1 | import riot from 'riot';
      2 | riot.tag2('app', '<h3>{opts.title}</h3> <p>{subtitle}</p> <ul> <li each="{list}">{name}</li> </ul>', 'app,[riot-tag="app"] { font-size: 2rem } app h3,[riot-tag="app"] h3 { color: #444 } app ul,[riot-tag="app"] ul { color: #999 }', '', function(opts) {
    > 3 |     import MyClass from "MyClass.js";
        |     ^
      4 | 
      5 |     var m = new MyClass();
      6 | 

So this looks like something that isn't actually supported due to the nature of the riot compilation process itself. If the import statement could be hoisted during the compilation then it would all work out, but that would need to be some sort of ast transform which is not simple to do.

You could get modules to work in tags by using CommonJS as the module format here instead. That would work out because CommonJs requires can appear anywhere in the code. Although it's not a very forward-thinking approach.

Perhaps just use a dynamic System.import('x').then.....

@GianlucaGuarini
Copy link
Member

Have you guys checked the import on rollup? Maybe this should solve the problem https://github.com/riot/examples/tree/gh-pages/rollup#use-imported-modules

@guybedford
Copy link

@GianlucaGuarini awesome, I can confirm that the plugin adjustment works with manual hosting of that import statement against the example app. It would definitely be worth sending that adjustment as a PR to the systemjs-riot plugin then, as it also brings Rollup support with it as well.

@GianlucaGuarini
Copy link
Member

the problem here is that the es6 modules can work only if located in the main scope.
When you do:

<tag>
   <script>
     import baz from './baz'
     this.foo = 'bar'
   </script>
</tag>

This gets translated by the riot compiler into:

riot.tag2('tag','', function(opts) {
   import baz from './baz' // import here is not allowed
   this.foo = 'bar'
})

I think that using all the import outside of the tags is a good workaround for now

@josephrocca
Copy link
Author

Thanks for your investigations Guy and Gianluca! So just to confirm, the current approach to getting riot and JSPM working together is by importing outside the tag as follows:

import myModule from 'myModule';
<my-tag>
  <script>
    this.root.innerHTML = myModule(opts.content); //using myModule
  </script>
</my-tag>

And editing this line to:

load.metadata.format = 'esm';

Is that correct? I'm not able to test this myself just now but will tomorrow.

@nikek
Copy link

nikek commented Apr 4, 2016

I figured I share my experiment as it is related: https://github.com/nikek/modern-riot
This is a starting point for using riot tags and rollup as ES6 module loader. I chose to use gulp, as the rollup-babel-plugin was using babel-6 which was too slow. The gulp way with babel 5.8 was fast enough to make an OK development feedback loop without having to wait a few seconds after each code update.

It's small and works pretty well except for the case I mention under the known issues topic in the readme. Rollup complains about "duplication of imports in the same file", which I still haven't had time to debug yet. Hope it helps someone.

@GianlucaGuarini
Copy link
Member

I will close this issue because the es6 modules specs do not allow the import of scripts in nested functions. My workaround is enough to solve the issue

ffflabs added a commit to HuasoFoundries/systemjs-riot that referenced this issue Jun 3, 2016
aMarCruz pushed a commit to riot/compiler that referenced this issue Aug 30, 2016
Add support for es6 `import` statements. Thanks to @kuashe!

Related to [riot#1715](riot/riot#1715), [riot#1784](riot/riot#1784), and [riot#1864](riot/riot#1864).
@aMarCruz
Copy link
Contributor

aMarCruz commented Aug 30, 2016

Please test with the riot-compiler v2.5.4

Now the entities options includes all the imports as a multiline string.
See the Guide.

@shyamchandranmec
Copy link

@aMarCruz @GianlucaGuarini I am also trying to load a tag dynamically and then mount it using jspm.

System.import("new-tag.js").then(//Mount the tag and update)

The tag is getting loaded in the DOM but its content is empty. It works If I put the import statement at the top of the module. But in this way I will have to load all the javascript file at once. I want to load these tags on demand.
Any way to solve this issue?

@aMarCruz
Copy link
Contributor

aMarCruz commented Sep 3, 2016

@shyamchandranmec , I don't know about jspm but from the specs System.import should to work anyware in the code (ref), so I think this is not a riot issue, looks like System.import() is not getting the function generated by riot.
Please review the ouput of System.import in DevTools.

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

No branches or pull requests

7 participants