geelen / jspm-loader-jsx

A hot-reloading JSX plugin for JSPM
21 stars 2 forks source link

es7 class properties causing error #2

Closed chrisdwheatley closed 9 years ago

chrisdwheatley commented 9 years ago

Following on from an issue I raised earlier in jspm-server I've amended my dev workflow somewhat as suggested by @kevrom and seem to have hit an issue with es7 class properties.

es7 class properties are available via babel stage 0, which I have enabled in config.js:

"babelOptions": {
    "stage": 0,
    "optional": [
      "runtime"
    ]
  },

I'm importing using the plugin from the html, e.g. System.import('index.jsx!') and then within that file setting class properties, e.g.

export default class Foo {
  bar = true
}

I'm then seeing the error Uncaught (in promise) Error: Parse Error: Line 2: Unexpected token = in the console, see screenshot below.

screen shot 2015-06-29 at 20 49 37

I created an example repo highlighting the issue. Just clone it & cd into it, jspm install then jspm-server to see three examples. js.html shows class properties working without this plugin, jsx.html shows the issue and finally jsx-no-classproperties.html shows the file loading correctly without the class properties set.

Sorry for the barrage of info, hopefully this can help debug. I'm happy to dig into the code with some guidance however when I took a quick look at the code nothing stood out to me. The size of the codebase suggest it may be an issue with one of the dependencies although that's a total guess...

kevrom commented 9 years ago

I actually ran into the same issue very recently. I'm gonna try and find the root of it.

On 3:10PM, Mon, Jun 29, 2015 Chris Wheatley notifications@github.com wrote:

Following on from an issue I raised earlier in jspm-server https://github.com/geelen/jspm-server/issues/18 I've amended my dev workflow somewhat as suggested by @kevrom https://github.com/kevrom and seem to have hit an issue with es7 class properties https://gist.github.com/jeffmo/054df782c05639da2adb.

es7 class properties are available via babel stage 0, which I have enabled in config.js:

"babelOptions": { "stage": 0, "optional": [ "runtime" ] },

I'm importing using the plugin from the html, e.g. System.import('index.jsx!') and then within that file setting class properties, e.g.

export default class Foo { bar = true }

I'm then seeing the error Uncaught (in promise) Error: Parse Error: Line 2: Unexpected token = in the console, see screenshot below.

[image: screen shot 2015-06-29 at 20 49 37] https://cloud.githubusercontent.com/assets/894092/8417098/3bae29cc-1ea2-11e5-8625-e1ec11502a59.png

I created an example repo https://github.com/swirlycheetah/jspm-systemjs-jsx-classproperties highlighting the issue. Just clone it & cd into it, jspm install then jspm-server to see three examples. js.html shows class properties working without this plugin, jsx.html shows the issue and finally jsx-no-classproperties.html shows the file loading correctly without the class properties set.

Sorry for the barrage of info, hopefully this can help debug. I'm happy to dig into the code with some guidance however when I took a quick look at the code nothing stood out to me. The size of the codebase suggest it may be an issue with one of the dependencies although that's a total guess...

— Reply to this email directly or view it on GitHub https://github.com/geelen/jspm-loader-jsx/issues/2.

chrisdwheatley commented 9 years ago

Awesome, thanks for your help @kevrom.

danieldunderfelt commented 9 years ago

Awesome, was just going to report this! @kevrom any progress?

kevrom commented 9 years ago

I haven't actually. I've been swamped, unfortunately.

danieldunderfelt commented 9 years ago

Oh well, thanks for your efforts in any case!

capaj commented 9 years ago

@danieldunderfelt this is because this plugin uses react-tools instead of babel to transform your code into es5 runnable by the browser. I don't know why @geelen would choose react-tools instead of babel, but it seems to me that this choice is not very wise. In the very least it increases the amount of scripts your app needs to even start running. Babel supports JSX out of the box, but for some reason es6-module loader.src.js disables it by default. Why is it disabling react support @guybedford ? So I think it should be possible to make it work with Babel, if we supply an option to Babel such as:

babelOptions: {
   blacklist: []
}
capaj commented 9 years ago

@swirlycheetah @kevrom @danieldunderfelt so I tried to set the blacklist to empty array and renamed my jsx files to js, also changed requires so that I require them as regular js files and it works. Obviously without any plugin, so live-reload does not work as well. @geelen I really think react-rools should GTFO. They are deprecated anyway. Babel FTW!

kevrom commented 9 years ago

Good catch. So I just stripped react-tools out to test and it works fine.

export let translate = load => {
  let snippet = BUILD_MODE ? '' : reexportHotVersionSnippet( classNameFromFilename( load.metadata.pluginArgument ) ),
    output = 'export let __hotReload;' + load.source + snippet;
  load.source = output;
}

I'm using the latest System.js beta, so I'm not 100% sure this will work in the latest stable. But the reason this works is because System.js will call its normal translate hook after doing this one. So there's no need to tie into any specific transpiler.

capaj commented 9 years ago

@kevrom you should do a PR. I just tested with jspm 0.15.7 and it works as well. Don't forget to remove "react-tools": "npm:react-tools@^0.13.1", from package.json.

kevrom commented 9 years ago

I will go ahead and do that.

geelen commented 9 years ago

I just copied the existing loader code, which used react-tools. No idea why System disabled JSX by default, but happy to go ahead and drop it. Glad it'll solve this issue.