Noderize icon indicating copy to clipboard operation
Noderize copied to clipboard

format and test scripts generate SyntaxErrors

Open sc0ttwad3 opened this issue 6 years ago • 23 comments

Seems reproducible with any new noderize app create, even after successful build/watch, initially running...

yarn format generates a SyntaxError via @noderize\scripts\node_modules\.bin\prettier and args handling? yarn test generates a SyntaxError via @noderize\scripts\node_modules\.bin\jest and args handling?

λ yarn format
yarn run v1.5.1
$ noderize-scripts format
[INFO] Formatting...
c:\projects\typescript\first-noderize\node_modules\@noderize\scripts\node_modules\.bin\prettier:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at new Script (vm.js:51:7)
    at createScript (vm.js:136:10)
    at Object.runInThisContext (vm.js:197:10)
    at Module._compile (module.js:613:28)
    at Object.Module._extensions..js (module.js:660:10)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:501:12)
    at Function.Module._load (module.js:493:3)
    at Function.Module.runMain (module.js:690:10)
    at startup (bootstrap_node.js:194:16)
[WARN] Done formatting!
Done in 1.67s.
λ yarn test
yarn run v1.5.1
$ noderize-scripts test
[INFO] Testing...
The system cannot find the path specified.
c:\projects\typescript\first-noderize\node_modules\@noderize\scripts\node_modules\.bin\jest:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at new Script (vm.js:51:7)
    at createScript (vm.js:136:10)
    at Object.runInThisContext (vm.js:197:10)
    at Module._compile (module.js:613:28)
    at Object.Module._extensions..js (module.js:660:10)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:501:12)
    at Function.Module._load (module.js:493:3)
    at Function.Module.runMain (module.js:690:10)
    at startup (bootstrap_node.js:194:16)
Done in 1.75s.

sc0ttwad3 avatar Mar 20 '18 00:03 sc0ttwad3

This seems to be a Windows-only problem.

This is odd since basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')") never appears in Noderize itself, only when calling other scripts (Prettier and Jest) using child_process.fork.

Could you provide more information on your set up?

Cretezy avatar Mar 20 '18 02:03 Cretezy

Your right it's the windows shell lacking shebang support. Is it that problem where yarn/npm ends up treating/running the shell script as javascript?

It looks like create-react-app and create-react-app-typescript are utilizing node-cross-spawn to handle this issue, but that's just a quick look. I'd have to look deeper into what they're doing.

Pretty standard Windows 10 latest build, lots of ram, fast SSDs, on this Spectre x360 laptop. Specific environment I'm using...

λ cmd --version
Microsoft Windows [Version 10.0.17123.1]
(c) 2017 Microsoft Corporation. All rights reserved.

yarn versions v1.5.1
{ yarn: '1.5.1',
  myapp: '0.1.0',
  http_parser: '2.7.0',
  node: '9.8.0',
  v8: '6.2.414.46-node.21',
  uv: '1.19.2',
  zlib: '1.2.11',
  ares: '1.13.0',
  modules: '59',
  nghttp2: '1.29.0',
  napi: '2',
  openssl: '1.0.2n',
  icu: '60.2',
  unicode: '10.0',
  cldr: '32.0.1',
  tz: '2017c' }

sc0ttwad3 avatar Mar 20 '18 03:03 sc0ttwad3

Ah, mimssing shebang support makes lots of sense. I'll work on a fix, give me a few minutes :+1:

Cretezy avatar Mar 20 '18 03:03 Cretezy

Sure thing.

Just reading the code/docs for node-cross-spawn. Specifically mentions node problems when using spawn on Windows: "Has an issue with command shims (files in node_modules/.bin/), where arguments with quotes and parenthesis would result in invalid syntax error". Looks promising.

sc0ttwad3 avatar Mar 20 '18 03:03 sc0ttwad3

https://github.com/Cretezy/Noderize/commit/0f6a90f539c94f81d3c2e2649156c4122be33534

Waiting for tests to pass then publishing as v0.4.4.

Cretezy avatar Mar 20 '18 03:03 Cretezy

Published as v0.4.4, thank you for the report and the library suggestion!

Cretezy avatar Mar 20 '18 03:03 Cretezy

I tried out 0.4.5 this afternoon (nice work) and can confirm all scripts work without any issues on linux systems for me.

format is still problematic on Widows and again giving the usual syntax error, same culprit, I suspect, the spawn issues node has on windows. Looks like cross-spawn is there now.

Some typo maybe in the format script? I'll take a quick look...

λ yarn format
yarn run v1.5.1
$ noderize-scripts format
[INFO] Formatting...
c:\projects\typescript\myapp\node_modules\@noderize\scripts\node_modules\.bin\prettier:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at new Script (vm.js:51:7)
    at createScript (vm.js:136:10)
    at Object.runInThisContext (vm.js:197:10)
    at Module._compile (module.js:613:28)
    at Object.Module._extensions..js (module.js:660:10)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:501:12)
    at Function.Module._load (module.js:493:3)
    at Function.Module.runMain (module.js:690:10)
    at startup (bootstrap_node.js:194:16)
[WARN] Done formatting!
Done in 1.94s.

sc0ttwad3 avatar Mar 20 '18 19:03 sc0ttwad3

Reopening this issue. Does this also happen on the test script?

Cretezy avatar Mar 21 '18 01:03 Cretezy

Related issues:

  • https://github.com/facebook/jest/issues/3750
  • https://github.com/yarnpkg/yarn/issues/4059

Can you try running (from your base directory) node node_modules\@noderize\scripts\node_modules\.bin\prettier and seeing if it works.

Cretezy avatar Mar 21 '18 02:03 Cretezy

Sure. I checked and prettier works fine...

λ node_modules\@noderize\scripts\node_modules\.bin\prettier --version
1.11.1

In another noderize generated app, I'm using a quick work around. I've added a script to the noderize generated app's package.json file, called format:js to invoke prettier:

"format:js": "prettier --trailing-comma es5 --single-quote --write \"./src/**/*.js\"",

It's manual, yarn format:js but prettier runs correctly even with configuration command-line options (I haven't checked alternatively using a .prettier file yet). Useful for me as a way to check/test the results of different options at the moment.

sc0ttwad3 avatar Mar 21 '18 15:03 sc0ttwad3

The test script seems to have the same issue. Syntax error when run as yarn test. However, it too, runs correctly with a fully qualified path:

c:\projects\typescript\thirdapp
λ node_modules\@noderize\scripts\node_modules\.bin\jest --version
v22.4.2

c:\projects\typescript\thirdapp
λ node_modules\@noderize\scripts\node_modules\.bin\jest
No tests found
In C:\projects\typescript\thirdapp
  3 files checked.
  testMatch: **/__tests__/**/*.js?(x),**/?(*.)(spec|test).js?(x) - 0 matches
  testPathIgnorePatterns: \\node_modules\\ - 3 matches
Pattern:  - 0 matches

c:\projects\typescript\thirdapp
λ yarn test
yarn run v1.5.1
$ noderize-scripts test
[INFO] Testing...
The system cannot find the path specified.
c:\projects\typescript\thirdapp\node_modules\@noderize\scripts\node_modules\.bin\jest:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^

SyntaxError: missing ) after argument list
    at new Script (vm.js:51:7)
    at createScript (vm.js:136:10)
    at Object.runInThisContext (vm.js:197:10)
    at Module._compile (module.js:613:28)
    at Object.Module._extensions..js (module.js:660:10)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:501:12)
    at Function.Module._load (module.js:493:3)
    at Function.Module.runMain (module.js:690:10)
    at startup (bootstrap_node.js:194:16)
Done in 2.28s.

sc0ttwad3 avatar Mar 21 '18 15:03 sc0ttwad3

Prettier handling command-line arguments on Windows from npm/yarn scripts has given me problems in the past. For some reason it seems to frequently not operate the way it does when called manually (more so than other tools). Are you seeing the same issues on Windows? Getting the same misleading SyntaxError output? BTW, the scripts are fine when running under bash in the Windows Subsystem for Linux (wsl) configured for Ubuntu as well, I forgot to mention.

Anyway, I've started investigating the ways people have modified react-scripts for Typescript and I have time the next couple of days for learning more about this area and cross-platform issues in general.

I've also been looking at some clever code in the typescript-starter project for handling command-line configuration pre-generation, and importantly, like this one, the option of using TypeScript or not.

But I'm not crazy about question answers instead of options on the command-line when configuring for the output project generation (like yeoman generators do) so I still like the approach of react-scripts like in this project. I'm looking at customized versions like custom-react-scripts-ts as well.

sc0ttwad3 avatar Mar 22 '18 02:03 sc0ttwad3

This specific bug is pretty weird. I'm not very well versed in Windows, so would need to re-setup my Windows workstation to try to debug this. If anyone has any experience with this, please let me know, and we can work together on fixing this!

Cretezy avatar Mar 22 '18 05:03 Cretezy

I have time today and tomorrow to look into it. and can let you know if I find a resolution.

sc0ttwad3 avatar Mar 22 '18 14:03 sc0ttwad3

The core issue seems to come down to npm assuming the command passed to it is a JS file--somewhere in the complicated script calling script decomposition into commands and args at work here--this is however not true on Windows where npm wraps bin files in a .cmd file.

Since npm can not parse .cmd files--where our SyntaxError: issue is coming from--you need to reference the bin file manually in the package.json scripts section as part of a script.

And in order to use this cross platform (i.e. Linux, Mac and Windows), you need to change the path from the form:

node_modules/.bin/<some_name>

to the full location and adding the .js file extension:

node_modules/<some_name>/bin/<some_name>.js

At least that's my current understanding of the issue.

sc0ttwad3 avatar Mar 24 '18 17:03 sc0ttwad3

Yeah, that was my thinking too. I choose the .bin directory because it's a more generalized way to get the bins, and is package-agonistic. I think I'll have to force the use of each, or use there APIs (easy with Jest, less so with Prettier last time I checked).

If you wanna code up a solution, let me know, else I'll have a fix working within a day or two (busy with some other stuff right now, sorry!)

-------- Original Message -------- On Mar 24, 2018, 13:20, Scott Wade wrote:

The core issue seems to come down to npm assuming the command passed to it is a JS file--somewhere in the complicated script calling script decomposition into commands and args at work here--this is however not true on Windows where npm wraps bin files in a .cmd file.

Since npm can not parse .cmd files--where our SyntaxError: issue is coming from--you need to reference the bin file manually in the package.json scripts section as part of a script.

And in order to use this cross platform (i.e. Linux, Mac and Windows), you need to change the path from the form:

node_modules/.bin/<some_name>

to the full location and adding the .js file extension:

node_modules/<some_name>/bin/<some_name>.js

At least that's my current understanding of the issue.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

Cretezy avatar Mar 24 '18 17:03 Cretezy

What setup do you recommend for running a development version of Noderize locally to work with? I'm having a problem with setting up a good debug workflow. Currently, I'm jumping through too many hoops and manual operations.

And then how do you get yarn create noderize new-app to use the local development version of noderize to test your changes?

sc0ttwad3 avatar Mar 24 '18 17:03 sc0ttwad3

I would say clone this main repo, go in scripts and run yarn bootstrap, then you can "test" on the 'create` package (so build scripts, and run the scripts in create).

To recap:

  • clone & cd
  • yarn
  • cd into packages/scripts, yarn bootstrap
  • do changes, yarn build (you can actually watch too)
  • cd in packages/create, yarn format or whichever command you want to test

I definitely need to make a CONTRIBUTING.md file. Will do soon!

Cretezy avatar Mar 24 '18 17:03 Cretezy

Thanks! I'll be able to look into this again tonight.

sc0ttwad3 avatar Mar 26 '18 19:03 sc0ttwad3

The yarn bootstrap step fails for me. The solution is probably right in front of me and I'm just not seeing it. But, I can't seem to find a bootstrap entry in the packages/scripts directory package.json file, for yarn to run, or an entry anywhere really. I've search the entire project repo. Then I even searched, as a remote possibility, for a bootstrap.js script buried somewhere, none found as expected, however.

sc0ttwad3 avatar Mar 27 '18 17:03 sc0ttwad3

My bad, try prepack

Cretezy avatar Mar 27 '18 18:03 Cretezy

@sc0ttwad3 if you are still working on this, I've created a CONTRIBUTING.md to help get started.

Cretezy avatar Apr 01 '18 00:04 Cretezy

@cretezy Thanks. Havent had as much time for anything past work, but with prepack working, Im looking at a couple new things I ran across for the cross platform issues. I get anywhere with it, Ill let you know.

sc0ttwad3 avatar Apr 02 '18 14:04 sc0ttwad3