Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

Version 0.8.0 is broken on Windows: SyntaxError: Unexpected token < #33

Closed
mpawlow opened this issue May 16, 2015 · 6 comments · Fixed by #35
Closed

Version 0.8.0 is broken on Windows: SyntaxError: Unexpected token < #33

mpawlow opened this issue May 16, 2015 · 6 comments · Fixed by #35

Comments

@mpawlow
Copy link

mpawlow commented May 16, 2015

Problem

Version 0.8.0 is broken on Windows.
See Error below.

Steps To Reproduce

  1. Implement the basic example
  2. Access the '/' route
    • BUG: Error occurs
    • WORK-AROUND: Use back leveled version: 0.7.2.

Error

            <DefaultLayout title={this.props.title}>
            ^
SyntaxError: Unexpected token <
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Module._extensions..js (module.js:478:10)
    at Object.require.extensions.(anonymous function) [as .jsx] (C:\...\node_modules\express-react-views\node_modules\babel\node_modules\babel-core\lib\babel\api\register\node.js:161:7)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at View.renderFile [as engine] (C:\...\node_modules\express-react-views\index.js:37:23)
    at View.render (C:\...\node_modules\express\lib\view.js:93:8)

Issue Duplicate?

#32

Node Package Versions

"express-react-views": "0.8.0",
"react": "0.13.3"

OS Version

Windows 7 Pro SP1 (64-bit)

@zpao
Copy link
Member

zpao commented May 16, 2015

Ok, so the problem is going to be how we register the transform. The big change in 0.8 was to switch to babel, which meant that we changed how we detected what needed to be transformed.

Specifically we now build up a regex and pass that into babel. AFAIK this was only tested on *nix so presumably we need to either build a better regex or there's bug in babel (I'm assuming it's our problem though).

I don't have a Windows machine super handy but this will hopefully be a pretty small fix. Would you or @gocreating be willing to help out and do a little bit of detective work to make the regex match what's being required? (might need to do options.settings.replace('\', '\\'), some logging of the regex being built and the filename should be enlightening)

@gocreating
Copy link

@zpao excuse me, I am not familiar with regex and react, and I think I cannot help you in the meantime.
This is a great project, and I take it as part of my seed project :)

@kohei-takata
Copy link

I use node-jsx and the error did not happen.

I run npm install node-jsx and added require('node-jsx').install(); above the router.

@xuteng
Copy link

xuteng commented May 18, 2015

I got the same problem , To fix it ,I readed the Babel's docs and finded that the 'only' requires an array of glob paths, so I modified the index.js

var onlyArry = [];
  onlyArry.push(options.settings.views);
  moduleDetectRegEx = new RegExp('^' + options.settings.views);
  require('babel/register')({
    only: onlyArry
  });
  registered = true;

It fixed.

@zpao
Copy link
Member

zpao commented May 18, 2015

@xuteng - Hmm, http://babeljs.io/docs/usage/require/ indicates it takes a regex. And actually looking at the src (https://github.com/babel/babel/blob/master/src/babel/api/register/node.js#L154) it looks like both will work and it'll convert to an array and regex from that. So I think all that you're really doing is creating a regex without the leading ^.

Can you log what options.settings.views and filename are? My concern with leaving off part of the regex is that we'll inadvertently catch files outside the view path.

@xuteng
Copy link

xuteng commented May 19, 2015

The options.settings.views and filename are:
image
Thank you for pointing out the mistake.
However the regex I got is (?:(?=.)d:\/(?=.)work\/(?=.)newWork\/(?=.)views) at last when passed an array at https://github.com/babel/babel/blob/master/src/babel/util.js#L99

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

Successfully merging a pull request may close this issue.

5 participants